From ce9f1a89801b2f7df4ea02689d82e8d48b02c4c5 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Mon, 6 Jul 2026 19:51:44 +0300 Subject: [PATCH] test(converter): nightly property cron + env knobs + counterexample README (#351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Items 1 & 2 of the #351 remainder. Item 1 — the flat/nested generative property tests now read SEED and NUM_RUNS from PROPERTY_SEED / PROPERTY_NUM_RUNS (via an envInt helper that honors an explicit 0 and falls back to the current defaults — 20250705/300 flat, /100 nested — on unset/empty/non-numeric). New .github/workflows/nightly-property.yml runs the generative suite daily (and on workflow_dispatch) with a random seed and NUM_RUNS≈5000; on failure it files a Gitea issue containing fast-check's shrunk counterexample (jq-escaped, dedup'd by title). No build step — the suite imports the converter from src/, so a tsc error can't masquerade as a property failure. Item 2 — new packages/prosemirror-markdown/README.md documents the counterexample process (surface -> shrink -> permanent fixture in test/fixtures/counterexamples/ + counterexamples.test.ts -> fix the converter, never weaken a property; maintainer-approved ACCEPTED/allowlist entries carry a reason) plus the two golden layers, the coverage allowlist, and how to run with the env knobs. Linked from AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/nightly-property.yml | 122 ++++++++++++++++++ AGENTS.md | 2 +- packages/prosemirror-markdown/README.md | 105 +++++++++++++++ .../flat-roundtrip.property.test.ts | 22 +++- .../nested-roundtrip.property.test.ts | 12 +- 5 files changed, 253 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/nightly-property.yml create mode 100644 packages/prosemirror-markdown/README.md diff --git a/.github/workflows/nightly-property.yml b/.github/workflows/nightly-property.yml new file mode 100644 index 00000000..32ed103b --- /dev/null +++ b/.github/workflows/nightly-property.yml @@ -0,0 +1,122 @@ +name: Nightly property fuzz + +# The daily heavy property run for the ProseMirror<->Markdown converter +# (packages/prosemirror-markdown). The PR/CI test run keeps NUM_RUNS modest to +# stay under budget; this cron cranks it up (~5000) with a RANDOM seed to hunt +# for deeper round-trip counterexamples than a fixed-seed PR run can reach. +# +# Counterexample -> fixture workflow: when a run fails, fast-check prints the +# SHRUNK minimal counterexample plus the reproducing seed. This job files a +# Gitea issue containing that seed + counterexample. A human then commits the +# shrunk doc as a PERMANENT fixture under +# packages/prosemirror-markdown/test/fixtures/counterexamples/ with a case in +# counterexamples.test.ts, and FIXES the converter (never weakens a property to +# hide the bug). See packages/prosemirror-markdown/README.md. + +on: + schedule: + # 03:00 UTC daily. + - cron: '0 3 * * *' + workflow_dispatch: + inputs: + num_runs: + description: 'fast-check runs per property' + required: false + default: '5000' + seed: + description: 'fast-check seed (empty = random)' + required: false + default: '' + +permissions: + contents: read + issues: write + +jobs: + property-fuzz: + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up pnpm + uses: pnpm/action-setup@v4 + + - name: Set up Node + uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + # No build step: the generative suite imports the converter from src/ + # directly (e.g. `from '../../src/lib/markdown-converter.js'`), so it runs + # against source without the package's build/. Skipping the build also + # keeps a tsc build error from masquerading as a property-test failure and + # filing a bogus counterexample issue. + - name: Resolve seed and run count + id: params + run: | + set -euo pipefail + SEED="${{ inputs.seed }}" + # Empty seed (cron, or a dispatch that left it blank) -> random. Combine + # two RANDOMs so the seed spans more than RANDOM's 0..32767 range. + [ -z "$SEED" ] && SEED=$(( (RANDOM << 15) | RANDOM )) + NUM_RUNS="${{ inputs.num_runs || '5000' }}" + echo "seed=$SEED" >> "$GITHUB_OUTPUT" + echo "num_runs=$NUM_RUNS" >> "$GITHUB_OUTPUT" + echo "Property fuzz: PROPERTY_SEED=$SEED PROPERTY_NUM_RUNS=$NUM_RUNS" + + - name: Run generative property suite + env: + PROPERTY_SEED: ${{ steps.params.outputs.seed }} + PROPERTY_NUM_RUNS: ${{ steps.params.outputs.num_runs }} + run: | + set -o pipefail + pnpm --filter @docmost/prosemirror-markdown exec \ + vitest run test/generative/ 2>&1 | tee property-output.txt + + # On failure fast-check has already printed the shrunk counterexample and + # its reproducing seed into property-output.txt. File a Gitea issue with + # the seed, run count and the tail of that output so a human can turn the + # counterexample into a permanent fixture (see the header comment). + - name: File issue on failure + if: failure() + env: + SEED: ${{ steps.params.outputs.seed }} + NUM_RUNS: ${{ steps.params.outputs.num_runs }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -uo pipefail + TITLE="Nightly property test failure (seed=${SEED})" + + # Best-effort dedup: skip if an open issue with the shared title prefix + # already exists. A failure of this check must NOT block issue creation. + EXISTING="" + if EXISTING=$(curl -sS \ + -H "Authorization: token ${GITHUB_TOKEN}" \ + "${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}/issues?state=open&limit=100"); then + if printf '%s' "$EXISTING" \ + | node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{let a;try{a=JSON.parse(s)}catch{process.exit(1)}if(!Array.isArray(a))process.exit(1);process.exit(a.some(i=>typeof i.title==="string"&&i.title.startsWith("Nightly property test failure"))?0:1)})'; then + echo "An open 'Nightly property test failure' issue already exists — skipping creation." + exit 0 + fi + fi + + # Build the JSON body with the test output SAFELY escaped (never hand- + # interpolate the counterexample into JSON). + BODY_TEXT=$(printf 'The nightly property fuzz run failed.\n\n- seed: `%s`\n- NUM_RUNS: `%s`\n- run: %s\n\nReproduce locally:\n\n```\nPROPERTY_SEED=%s PROPERTY_NUM_RUNS=%s pnpm --filter @docmost/prosemirror-markdown exec vitest run test/generative/\n```\n\nfast-check shrinks the failure to a minimal counterexample. Commit it as a permanent fixture under `packages/prosemirror-markdown/test/fixtures/counterexamples/` + a case in `counterexamples.test.ts`, then FIX the converter (do not weaken a property). See `packages/prosemirror-markdown/README.md`.\n\nTail of the test output (contains the shrunk counterexample):\n\n```\n%s\n```\n' \ + "$SEED" "$NUM_RUNS" "$RUN_URL" "$SEED" "$NUM_RUNS" "$(tail -n 120 property-output.txt)") + + jq -n --arg title "$TITLE" --arg body "$BODY_TEXT" \ + '{title: $title, body: $body}' > payload.json + + curl -sS -X POST \ + "${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}/issues" \ + -H "Authorization: token ${GITHUB_TOKEN}" \ + -H 'Content-Type: application/json' \ + -d @payload.json diff --git a/AGENTS.md b/AGENTS.md index 30c542ba..0e3a0a76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -311,7 +311,7 @@ The API server is a Fastify app with a global `/api` prefix (`main.ts` excludes ### Client structure Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirrors the server domains: `page`, `space`, `comment`, `ai-chat`, `editor`, …). Conventions: - **TanStack Query** for server state (one `queries/` file per feature), **Jotai** atoms for local/shared UI state, **Mantine 8** + CSS modules (`*.module.css`) + `postcss-preset-mantine` for UI. -- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, and `apps/server` (#345) — do NOT reintroduce a per-package copy. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. +- The editor is Tiptap; shared node/mark extensions live in `packages/editor-ext` and are imported by **both the client and the server** (collaboration, schema, `canonicalizeFootnotes`) — editor schema changes often need to be made in `editor-ext`, not just the client. Server-side markdown import/export no longer lives in `editor-ext`: it goes through the canonical converter (#345, see below). The ProseMirror↔Markdown converter and its Docmost schema mirror now live in a SINGLE package, `@docmost/prosemirror-markdown` (#293), consumed by `mcp`, `git-sync`, and `apps/server` (#345) — do NOT reintroduce a per-package copy. `editor-ext` is the upstream source of the Tiptap schema; the package's `docmost-schema.ts` mirrors it and a serializer-contract test (`packages/prosemirror-markdown/test/serializer-contract.test.ts`) guards the boundary (every schema node must have a converter case), so a drift surfaces as a failing test rather than silent divergence. For the converter's property-testing and counterexample→fixture process (P1–P4 invariants, the `PROPERTY_SEED`/`PROPERTY_NUM_RUNS` knobs, and the nightly fuzz workflow), see `packages/prosemirror-markdown/README.md`. - API access goes through `apps/client/src/lib/api-client.ts` (axios). The `@` alias maps to `apps/client/src`. - Runtime config is injected at build time by `vite.config.ts` via `define` (`APP_URL`, `COLLAB_URL`, `APP_VERSION`, …) — these come from the root `.env`, not from `import.meta.env`. diff --git a/packages/prosemirror-markdown/README.md b/packages/prosemirror-markdown/README.md new file mode 100644 index 00000000..0c60d7d6 --- /dev/null +++ b/packages/prosemirror-markdown/README.md @@ -0,0 +1,105 @@ +# @docmost/prosemirror-markdown + +The single, canonical **ProseMirror ↔ Markdown converter** plus the Docmost +schema mirror (#293/#345). Headless and framework-free: no React, no browser +runtime. There is exactly ONE copy of this converter in the repo, consumed by: + +- `packages/mcp` (the MCP server), +- `packages/git-sync` (two-way Git sync), +- `apps/server` (server-side markdown import/export, #345). + +`src/lib/docmost-schema.ts` **mirrors** the upstream Tiptap schema that lives in +`packages/editor-ext`. The mirror is not free-floating: `serializer-contract.test.ts` +guards the boundary — every schema node must have a converter case, so a drift +between `editor-ext` and this package surfaces as a failing test rather than a +silent divergence. + +## Why byte-stability matters + +Git sync exports a page to markdown, and re-imports it on the next pull. If +`export → import → export` is not **byte-stable**, every pull rewrites files +that nobody edited, and the user's history churns with phantom diffs. So the +converter is held to more than "it roughly round-trips": the second export pass +must be a byte-for-byte fixpoint. That is what the property suite below proves. + +## Two golden layers (do not mix them) + +1. **Corpus fixtures** — `test/fixtures/corpus/`. A fixed, hand-curated set of + representative documents (headings, marks, lists, tables, diagrams, columns, + details, mentions, …). These are the readable, deterministic "known-good" + snapshots. Edit them deliberately. + +2. **Generative property suite** — `test/generative/`. fast-check draws random + documents and asserts invariants over them. Two entry points: + - `flat-roundtrip.property.test.ts` — flat documents, attribute-level fuzzing. + - `nested-roundtrip.property.test.ts` — deeply nested structures. + + The invariants (**P1–P4**): + - **P1** — semantic round-trip: `mdToPm(pmToMd(doc))` is canonically equal to + `doc` (no data loss for the round-trip-supported space). + - **P2** — byte fixpoint: `pmToMd(mdToPm(pmToMd(doc))) === pmToMd(doc)` (the + first pass may normalize once; the second pass must be a fixpoint). + - **P3** — totality: neither converter throws; output is bounded. + - **P4** — parser fuzz totality: for ANY input string, `markdownToProseMirror` + does not throw and returns a schema-valid document. + + These invariants are kept **STRICT** — no `it.fails`, skip, or weakening. A + failure means the generator found a REAL converter bug. + +## The counterexample process (the DoD) + +This is the point of the generative layer. When a property run diverges: + +1. **A property run surfaces a divergence** (locally, in CI, or in the nightly + cron — see below). +2. **fast-check shrinks it** to a minimal, human-readable counterexample and + prints the reproducing seed. +3. **Commit the shrunk doc as a permanent fixture** under + `test/fixtures/counterexamples/`, with a matching case in + `counterexamples.test.ts`. The fixture stays forever, as a regression pin. +4. **FIX the converter** so the counterexample round-trips. **Never weaken a + property to hide the bug.** +5. If — and only if — a maintainer decides a particular markdown-representable + loss is genuinely acceptable, it is recorded as an **explicit ACCEPTED / + allowlist entry with a written reason**, not by silently relaxing an + invariant. + +### Attribute-coverage allowlist + +`flat-roundtrip.property.test.ts` maintains `ATTR_VALUE_FUZZ_ALLOWLIST`. The +suite asserts that every attribute in the live schema is EITHER value-fuzzed by +the generator OR explicitly listed in this allowlist. This forces any newly +added node attribute to be consciously classified — you cannot add an attr and +leave it silently un-exercised; the coverage test fails until you either fuzz it +or record why it is held out. + +## Running + +```sh +# The full package suite (corpus + generative + contract tests): +pnpm --filter @docmost/prosemirror-markdown test +``` + +The generative suite honours two env knobs (invalid/empty → falls back to the +default): + +| Env var | Default (flat) | Default (nested) | Meaning | +| -------------------- | -------------- | ---------------- | -------------------------------- | +| `PROPERTY_SEED` | `20250705` | `20250705` | fast-check seed (reproducibility) | +| `PROPERTY_NUM_RUNS` | `300` | `100` | runs per property | + +```sh +# Reproduce a specific counterexample seed with a bigger budget: +PROPERTY_SEED=12345 PROPERTY_NUM_RUNS=5000 \ + pnpm --filter @docmost/prosemirror-markdown exec vitest run test/generative/ +``` + +### Nightly cron + +`.github/workflows/nightly-property.yml` runs the generative suite every night +with `PROPERTY_NUM_RUNS` cranked to ~5000 and a **random** seed, to reach deeper +counterexamples than a fixed-seed PR run can. On failure it files a Gitea issue +containing the reproducing seed, the run count, and the tail of the output (the +shrunk counterexample), which kicks off the counterexample → fixture process +above. It can also be triggered manually (`workflow_dispatch`) with custom +`num_runs` / `seed`. diff --git a/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts b/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts index dfdd2b8c..08aa7ac2 100644 --- a/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts +++ b/packages/prosemirror-markdown/test/generative/flat-roundtrip.property.test.ts @@ -111,13 +111,21 @@ vi.setConfig({ testTimeout: 30000 }); // Fixed seed so every failure is reproducible; fast-check also prints the // shrunk counterexample. numRuns starts modest to keep CI under budget — the -// issue's CI target is ~300-500 per property; the nightly / PR 3 will crank -// this up further. Each property runs over the UNION (fc.oneof) of all flat -// node generators, so the runs are shared across node types (one test per -// property keeps the jsdom import cost and memory bounded — a per-generator × -// per-property matrix is ~200 heavy tests that OOMs the worker). -const SEED = 20250705; -const NUM_RUNS = 300; +// issue's CI target is ~300-500 per property. Both are overridable via the +// PROPERTY_SEED / PROPERTY_NUM_RUNS env vars (an invalid/empty value → NaN → +// falls back to the default below): the nightly cron +// (.github/workflows/nightly-property.yml) cranks NUM_RUNS to ~5000 with a +// random seed to hunt for deeper counterexamples. Each property runs over the +// UNION (fc.oneof) of all flat node generators, so the runs are shared across +// node types (one test per property keeps the jsdom import cost and memory +// bounded — a per-generator × per-property matrix is ~200 heavy tests that +// OOMs the worker). +// An unset/empty/non-numeric value falls back to the default; an explicit 0 is +// honored (a valid fast-check seed) — `Number(x) || default` would swallow it. +const envInt = (v: string | undefined, dflt: number): number => + v !== undefined && v !== '' && Number.isFinite(Number(v)) ? Number(v) : dflt; +const SEED = envInt(process.env.PROPERTY_SEED, 20250705); +const NUM_RUNS = envInt(process.env.PROPERTY_NUM_RUNS, 300); const P1_GENERATORS = buildGenerators('p1'); const FUZZ_GENERATORS = buildGenerators('fuzz'); diff --git a/packages/prosemirror-markdown/test/generative/nested-roundtrip.property.test.ts b/packages/prosemirror-markdown/test/generative/nested-roundtrip.property.test.ts index a5c773c0..d22dbf77 100644 --- a/packages/prosemirror-markdown/test/generative/nested-roundtrip.property.test.ts +++ b/packages/prosemirror-markdown/test/generative/nested-roundtrip.property.test.ts @@ -33,12 +33,20 @@ vi.setConfig({ testTimeout: 60000 }); // failure prints the shrunk minimal counterexample for triage. // --------------------------------------------------------------------------- -const SEED = 20250705; +// Both are overridable via the PROPERTY_SEED / PROPERTY_NUM_RUNS env vars (an +// invalid/empty value → NaN → falls back to the default below); the nightly +// cron (.github/workflows/nightly-property.yml) cranks NUM_RUNS up with a +// random seed to hunt for deeper counterexamples. +// An unset/empty/non-numeric value falls back to the default; an explicit 0 is +// honored (a valid fast-check seed) — `Number(x) || default` would swallow it. +const envInt = (v: string | undefined, dflt: number): number => + v !== undefined && v !== '' && Number.isFinite(Number(v)) ? Number(v) : dflt; +const SEED = envInt(process.env.PROPERTY_SEED, 20250705); // The nested walk builds far heavier docs than the flat suite (each P1/P2 run // parses the emitted markdown through jsdom), so keep the run count moderate to // hold runtime and worker memory in budget while still exercising deep // structures. P4 (cheap string parsing) runs at a higher count below. -const NUM_RUNS = 100; +const NUM_RUNS = envInt(process.env.PROPERTY_NUM_RUNS, 100); const pmToMd = (doc: unknown): string => convertProseMirrorToMarkdown(doc); const mdToPm = (md: string): Promise => markdownToProseMirror(md);