Merge pull request 'test(converter): хвост #351 — nightly property cron + README + запиненные баги + media-фазз' (#391) from feat/351-remainder into develop
Reviewed-on: #391
This commit was merged in pull request #391.
This commit is contained in:
@@ -0,0 +1,224 @@
|
||||
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 up total coverage with random seeds to hunt
|
||||
# for deeper round-trip counterexamples than a fixed-seed PR run can reach.
|
||||
#
|
||||
# WHY SHARDING: a single mega-run (~10000 fast-check runs) OOMs the vitest worker
|
||||
# (empirically ~1625 runs -> "JS heap out of memory", ~2GB) because heap
|
||||
# accumulates across the whole property run in one process. Instead this job runs
|
||||
# SHARDS fresh vitest processes, each a MODERATE per-shard count with a DISTINCT
|
||||
# derived seed, so total coverage ~= SHARDS x PER_SHARD_NUM_RUNS across processes
|
||||
# that never accumulate heap. On the first failing shard we stop and keep that
|
||||
# shard's output for triage.
|
||||
#
|
||||
# Counterexample -> fixture workflow: when a shard fails, fast-check prints the
|
||||
# SHRUNK minimal counterexample plus the reproducing seed. This job files a Gitea
|
||||
# issue containing that seed + counterexample ONLY when the output actually holds
|
||||
# a fast-check counterexample; an infra failure (OOM/tsc/install, no
|
||||
# counterexample) is filed under a DISTINCT title so it can never poison the
|
||||
# counterexample dedup. 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 SHARD (8 shards run in sequence)'
|
||||
required: false
|
||||
default: '600'
|
||||
seed:
|
||||
description: 'base fast-check seed (empty = random); shard i uses base+i'
|
||||
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 base seed and per-shard run count
|
||||
id: params
|
||||
# Dispatch inputs are read via env (NOT interpolated into the shell body)
|
||||
# to avoid script injection through a crafted input value.
|
||||
env:
|
||||
SEED_INPUT: ${{ inputs.seed }}
|
||||
NUM_RUNS_INPUT: ${{ inputs.num_runs }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
SEED="${SEED_INPUT:-}"
|
||||
# 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="${NUM_RUNS_INPUT:-}"
|
||||
[ -z "$NUM_RUNS" ] && NUM_RUNS=600
|
||||
echo "seed=$SEED" >> "$GITHUB_OUTPUT"
|
||||
echo "num_runs=$NUM_RUNS" >> "$GITHUB_OUTPUT"
|
||||
echo "Sharded property fuzz: BASE_SEED=$SEED PER_SHARD_NUM_RUNS=$NUM_RUNS SHARDS=8"
|
||||
|
||||
- name: Run generative property suite (sharded)
|
||||
id: fuzz
|
||||
env:
|
||||
BASE_SEED: ${{ steps.params.outputs.seed }}
|
||||
PER_SHARD_NUM_RUNS: ${{ steps.params.outputs.num_runs }}
|
||||
SHARDS: '8'
|
||||
run: |
|
||||
set -uo pipefail
|
||||
# Give each fresh process headroom, but rely on SHARDING (not a big heap)
|
||||
# to avoid OOM: a moderate per-shard count in a process that starts clean.
|
||||
export NODE_OPTIONS=--max-old-space-size=4096
|
||||
: > property-output.txt
|
||||
FAILED=0
|
||||
FAIL_SEED=""
|
||||
i=0
|
||||
while [ "$i" -lt "$SHARDS" ]; do
|
||||
SHARD_SEED=$(( BASE_SEED + i ))
|
||||
echo "=== shard $((i + 1))/$SHARDS: PROPERTY_SEED=$SHARD_SEED PROPERTY_NUM_RUNS=$PER_SHARD_NUM_RUNS ==="
|
||||
# tee OVERWRITES property-output.txt each shard; since we break on the
|
||||
# first failure, the file ends up holding exactly the failing shard's
|
||||
# output (which carries the shrunk counterexample + reproducing seed).
|
||||
if PROPERTY_SEED="$SHARD_SEED" PROPERTY_NUM_RUNS="$PER_SHARD_NUM_RUNS" \
|
||||
pnpm --filter @docmost/prosemirror-markdown exec \
|
||||
vitest run test/generative/ 2>&1 | tee property-output.txt; then
|
||||
echo "shard $((i + 1)) passed"
|
||||
else
|
||||
echo "shard $((i + 1)) FAILED (seed=$SHARD_SEED) — stopping; keeping its output"
|
||||
FAILED=1
|
||||
FAIL_SEED="$SHARD_SEED"
|
||||
break
|
||||
fi
|
||||
i=$(( i + 1 ))
|
||||
done
|
||||
echo "failed=$FAILED" >> "$GITHUB_OUTPUT"
|
||||
echo "fail_seed=$FAIL_SEED" >> "$GITHUB_OUTPUT"
|
||||
exit "$FAILED"
|
||||
|
||||
# A GENUINE counterexample: fast-check printed a shrunk minimal case and its
|
||||
# reproducing seed into property-output.txt. File a dedup-guarded issue whose
|
||||
# title prefix is UNIQUE to counterexamples, so an infra failure (handled by
|
||||
# the next step under a different title) can never poison this dedup.
|
||||
- name: File counterexample issue
|
||||
# always() is REQUIRED: the fuzz step exits nonzero on a failing shard,
|
||||
# so a bare `if:` (implicitly success() && ...) would skip this step
|
||||
# exactly when it must run. always() lets it run on the failure path.
|
||||
if: always() && steps.fuzz.outputs.failed == '1'
|
||||
env:
|
||||
FAIL_SEED: ${{ steps.fuzz.outputs.fail_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 }}
|
||||
TITLE_PREFIX: 'Nightly property counterexample'
|
||||
run: |
|
||||
set -uo pipefail
|
||||
# Discriminate counterexample vs infra failure by the fast-check
|
||||
# signature. No signature -> leave it to the infra-failure step.
|
||||
if ! grep -Eq 'Property failed after|Counterexample' property-output.txt; then
|
||||
echo "No fast-check counterexample signature — infra failure, handled by the next step."
|
||||
exit 0
|
||||
fi
|
||||
TITLE="${TITLE_PREFIX} (seed=${FAIL_SEED})"
|
||||
|
||||
# Best-effort dedup: skip if an open issue with the counterexample title
|
||||
# prefix already exists. A failure of this check must NOT block 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);const p=process.env.TITLE_PREFIX;process.exit(a.some(i=>typeof i.title==="string"&&i.title.startsWith(p))?0:1)})'; then
|
||||
echo "An open '${TITLE_PREFIX}' 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 'A nightly property fuzz SHARD failed with a fast-check counterexample.\n\n- failing shard seed: `%s`\n- NUM_RUNS (per shard): `%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' \
|
||||
"$FAIL_SEED" "$NUM_RUNS" "$RUN_URL" "$FAIL_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
|
||||
|
||||
# An INFRA failure (OOM, tsc, install) has NO counterexample signature. File
|
||||
# it under a DISTINCT title so it is visible but keeps the counterexample
|
||||
# dedup (above) uncontaminated — a real counterexample can still file even
|
||||
# while an infra issue is open.
|
||||
- name: File infra failure issue
|
||||
# always() is REQUIRED: the fuzz step exits nonzero on a failing shard,
|
||||
# so a bare `if:` (implicitly success() && ...) would skip this step
|
||||
# exactly when it must run. always() lets it run on the failure path.
|
||||
if: always() && steps.fuzz.outputs.failed == '1'
|
||||
env:
|
||||
FAIL_SEED: ${{ steps.fuzz.outputs.fail_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 }}
|
||||
TITLE_PREFIX: 'Nightly property run infra failure'
|
||||
run: |
|
||||
set -uo pipefail
|
||||
# Only file when there is NO counterexample signature (else the
|
||||
# counterexample step owns it).
|
||||
if grep -Eq 'Property failed after|Counterexample' property-output.txt; then
|
||||
echo "Counterexample present — owned by the counterexample step."
|
||||
exit 0
|
||||
fi
|
||||
TITLE="${TITLE_PREFIX} (seed=${FAIL_SEED})"
|
||||
|
||||
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);const p=process.env.TITLE_PREFIX;process.exit(a.some(i=>typeof i.title==="string"&&i.title.startsWith(p))?0:1)})'; then
|
||||
echo "An open '${TITLE_PREFIX}' issue already exists — skipping creation."
|
||||
exit 0
|
||||
fi
|
||||
fi
|
||||
|
||||
BODY_TEXT=$(printf 'A nightly property fuzz SHARD failed WITHOUT a fast-check counterexample (infra failure: OOM / build / install). This is NOT a converter round-trip bug.\n\n- failing shard seed: `%s`\n- NUM_RUNS (per shard): `%s`\n- run: %s\n\nInvestigate the run log (memory, dependency install, or a tsc/import error). The nightly counterexample dedup is intentionally separate from this issue.\n\nTail of the test output:\n\n```\n%s\n```\n' \
|
||||
"$FAIL_SEED" "$NUM_RUNS" "$RUN_URL" "$(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
|
||||
@@ -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`.
|
||||
|
||||
|
||||
@@ -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`.
|
||||
@@ -1006,6 +1006,8 @@ const Column = Node.create({
|
||||
width: {
|
||||
default: null,
|
||||
parseHTML: (el: HTMLElement) => {
|
||||
// Mirrors editor-ext (column.ts): width is a unitless flex-grow
|
||||
// number, so parse it to a Number for parity with the canonical schema.
|
||||
const value = el.getAttribute("data-width");
|
||||
return value ? parseFloat(value) : null;
|
||||
},
|
||||
|
||||
@@ -635,12 +635,23 @@ export function convertProseMirrorToMarkdown(
|
||||
.map((item: any) => processListItem(item, "-"))
|
||||
.join("\n");
|
||||
|
||||
case "orderedList":
|
||||
case "orderedList": {
|
||||
// Honor a non-1 `start`: CommonMark expresses it as the first marker
|
||||
// ("5." starts the list at 5), and marked parses that back into
|
||||
// <ol start="5">. Emitting `${start + index}.` keeps a start=5 list as
|
||||
// "5.","6.",… while a default (start=1) list stays "1.","2.",… See #351.
|
||||
// Only an INTEGER ≥ 2 is a valid explicit start; collapse everything else
|
||||
// (absent, non-number, 0, negative, fractional) to 1. A negative/fractional
|
||||
// start would otherwise emit an untokenizable marker (e.g. "-3.", "2.5.")
|
||||
// that marked cannot parse, re-importing the whole list as a PARAGRAPH.
|
||||
const raw = node.attrs?.start;
|
||||
const start = Number.isInteger(raw) && (raw as number) > 1 ? (raw as number) : 1;
|
||||
return nodeContent
|
||||
.map((item: any, index: number) =>
|
||||
processListItem(item, `${index + 1}.`),
|
||||
processListItem(item, `${start + index}.`),
|
||||
)
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
case "taskList":
|
||||
return nodeContent.map((item: any) => processTaskItem(item)).join("\n");
|
||||
@@ -1401,10 +1412,20 @@ export function convertProseMirrorToMarkdown(
|
||||
return `<ul>${children
|
||||
.map((li: any) => `<li>${blockChildrenToHtml(li)}</li>`)
|
||||
.join("")}</ul>`;
|
||||
case "orderedList":
|
||||
return `<ol>${children
|
||||
case "orderedList": {
|
||||
// Carry a non-1 `start` on the raw-HTML path (columns/spanned cells) via
|
||||
// the <ol start="N"> attribute, which the tiptap parser reads back into
|
||||
// attrs.start. A default (start=1) list stays a bare <ol>. See #351.
|
||||
// Use the SAME integer-≥-2 guard as the markdown path: a fractional start
|
||||
// would emit `start="2.5"` → parseInt→2 on import (path divergence), and a
|
||||
// 0/negative start is not a valid explicit start either.
|
||||
const raw = block.attrs?.start;
|
||||
const start = Number.isInteger(raw) && (raw as number) > 1 ? (raw as number) : 1;
|
||||
const startAttr = start > 1 ? ` start="${start}"` : "";
|
||||
return `<ol${startAttr}>${children
|
||||
.map((li: any) => `<li>${blockChildrenToHtml(li)}</li>`)
|
||||
.join("")}</ol>`;
|
||||
}
|
||||
case "codeBlock": {
|
||||
const lang = block.attrs?.language || "";
|
||||
// The code itself is element TEXT content (between <code> tags), so it
|
||||
|
||||
Vendored
-16
@@ -1,16 +0,0 @@
|
||||
{
|
||||
"_bug": "BUG #351: a `column` whose `width` is a percentage string (e.g. \"50%\") is NOT byte-stable across export->import->export (violates P2). The `column` schema's parseHTML does `parseFloat(getAttribute('data-width'))`, which silently drops the '%' unit and returns the NUMBER 50. So the first export emits data-width=\"50%\" but the re-import stores width=50, and the second export emits data-width=\"50\": md2 !== md1, a permanent GS-EDIT-REVERT churn (every git-sync pull rewrites the column width). The editor authors column widths as percentages, so this is a real data/round-trip defect. Fix belongs in src/lib/docmost-schema.ts column.width parseHTML (preserve the unit / keep the string), which is OUT OF SCOPE for this test-only PR and must be a separate, maintainer-approved change. This flat generator therefore keeps `column.width` frozen (never generates a non-default width).",
|
||||
"doc": {
|
||||
"type": "doc",
|
||||
"content": [
|
||||
{
|
||||
"type": "columns",
|
||||
"attrs": { "layout": "two_equal", "widthMode": "normal" },
|
||||
"content": [
|
||||
{ "type": "column", "attrs": { "width": "50%" }, "content": [{ "type": "paragraph", "content": [{ "type": "text", "text": "L" }] }] },
|
||||
{ "type": "column", "attrs": { "width": "50%" }, "content": [{ "type": "paragraph", "content": [{ "type": "text", "text": "R" }] }] }
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -37,14 +37,19 @@
|
||||
* tagged `// ACCEPTED:` inline. Freezing them is correct — there is
|
||||
* nothing to preserve in the target format.
|
||||
*
|
||||
* (b) PINNED BUG — the attribute IS representable in markdown but the
|
||||
* converter drops it anyway (a real defect). These are NOT silently
|
||||
* frozen: each is captured as a LOUD `it.fails` counterexample in
|
||||
* test/fixtures/counterexamples/ + counterexamples.test.ts, and the
|
||||
* freeze here only keeps the P1/P2 union green until a MAINTAINER rules
|
||||
* on accept-vs-fix (the epic guardrail reserves that call). These:
|
||||
* `column.width` (parseFloat drops `%`), `orderedList.start` (non-1
|
||||
* start renders as `1.`). Tagged `// PINNED-BUG:` inline.
|
||||
* (b) FIXED & VALUE-FUZZED — attributes that were once PINNED converter
|
||||
* bugs (representable in markdown but dropped) and are now FIXED in
|
||||
* src/, so they are value-fuzzed here at legal non-default values like
|
||||
* any healthy attr. `orderedList.start` (the non-1 start once rendered
|
||||
* as `1.`; the converter now emits the start marker / `<ol start="N">`)
|
||||
* and `column.width` (a unitless flex-grow number that round-trips via
|
||||
* parseFloat) are both fuzzed in OVERRIDES below. The former held-out
|
||||
* `it.fails` cases are gone; `ordered-list-start.json` +
|
||||
* counterexamples.test.ts now stand as PASSING regression pins (per the
|
||||
* epic guardrail, the minimal doc stays forever to guard re-regression).
|
||||
* The #351 media-family sizing attrs (image/video/youtube/pdf/drawio/
|
||||
* excalidraw/embed width/height/size/aspectRatio) are likewise fuzzed
|
||||
* now that they ride round-trip-safely in the per-node `<!--…-->` JSON.
|
||||
*
|
||||
* (c) DEFERRED-BUG — representable AND round-trips, frozen only because the
|
||||
* flat generator can't yet build a valid instance. Table
|
||||
@@ -64,8 +69,10 @@
|
||||
* number re-parses as a string), EXCEPT `embed.width/height` which the
|
||||
* embed schema keeps numeric — handled per-attr.
|
||||
*
|
||||
* Both PINNED-BUG attrs (`column.width` P2 churn, `orderedList.start` P1 loss)
|
||||
* are captured as committed `it.fails` counterexamples — NOT hidden here.
|
||||
* The two former PINNED-BUG attrs (`column.width` P2 churn, `orderedList.start`
|
||||
* P1 loss) are now FIXED and value-fuzzed; `ordered-list-start.json` in
|
||||
* counterexamples.test.ts is a permanent PASSING regression pin, not an
|
||||
* `it.fails` hold-out.
|
||||
*/
|
||||
import fc from 'fast-check';
|
||||
import { getSchema } from '@tiptap/core';
|
||||
@@ -120,6 +127,11 @@ interface AttrPolicy {
|
||||
const num = (...xs: number[]) => fc.constantFrom(...xs);
|
||||
const str = (...xs: string[]) => fc.constantFrom(...xs);
|
||||
const widthStr = str('120', '320', '640');
|
||||
// Media `aspectRatio`/`size` are stringified numerics too (converter emits
|
||||
// String(value)); the schema parseHTML reads them back as strings. Fuzz as
|
||||
// plausible numeric strings so they round-trip byte-stably like widthStr.
|
||||
const aspectRatioStr = str('1.5', '0.75', '1');
|
||||
const sizeStr = str('320', '640');
|
||||
|
||||
// The documented override table, keyed `type.attr`. Every entry is grounded in
|
||||
// the empirical converter probe (see flat-roundtrip.property.test.ts header).
|
||||
@@ -134,10 +146,10 @@ const OVERRIDES: Record<string, AttrPolicy> = {
|
||||
'heading.textAlign': { arb: str('center', 'right', 'justify') },
|
||||
'heading.indent': { frozen: true }, // ACCEPTED: no md representation
|
||||
// ── lists ────────────────────────────────────────────────────────────────
|
||||
// PINNED-BUG: markdown CAN express a non-1 start ("5."), but the converter
|
||||
// renders "1." and drops it -> P1 loss. See counterexamples.test.ts
|
||||
// (ordered-list-start.json). Frozen only until the maintainer rules accept-vs-fix.
|
||||
'orderedList.start': { always: true, frozen: true },
|
||||
// FIXED (#351): the converter now emits the start marker ("5." / <ol start="5">)
|
||||
// and it round-trips, so the start number is value-fuzzed. See
|
||||
// counterexamples.test.ts (ordered-list-start.json) for the regression pin.
|
||||
'orderedList.start': { always: true, arb: num(2, 3, 5, 42) },
|
||||
'orderedList.type': { frozen: true }, // ACCEPTED: a/A/i markers not expressible in GFM
|
||||
'taskItem.checked': { always: true, arb: fc.constant(true) }, // boolean, default false
|
||||
// ── codeBlock ────────────────────────────────────────────────────────────
|
||||
@@ -149,16 +161,57 @@ const OVERRIDES: Record<string, AttrPolicy> = {
|
||||
'image.title': { arb: letterPhraseArb },
|
||||
'image.width': { arb: widthStr, degen: '' },
|
||||
'image.height': { arb: widthStr, degen: '' },
|
||||
// #351 (this PR): media sizing/family attrs ride in the `<!--img {…}-->`
|
||||
// comment JSON via String(value); parseHTML reads them back as strings, so a
|
||||
// numeric string round-trips byte-stably (mirrors image.width).
|
||||
'image.size': { arb: sizeStr, degen: '' },
|
||||
'image.aspectRatio': { arb: aspectRatioStr },
|
||||
// caption is text carried verbatim in the comment JSON (mirrors image.alt).
|
||||
'image.caption': { arb: letterPhraseArb, degen: '' },
|
||||
'video.src': { noDefault: true, arb: urlArb, degen: '' },
|
||||
'video.alt': { arb: letterPhraseArb },
|
||||
'video.width': { arb: widthStr },
|
||||
'video.height': { arb: widthStr },
|
||||
// #351 (this PR): video sizing/align ride in the `<!--video {…}-->` comment.
|
||||
'video.size': { arb: sizeStr },
|
||||
'video.aspectRatio': { arb: aspectRatioStr },
|
||||
// align default "center" is dropped on export; fuzz non-center only.
|
||||
'video.align': { arb: str('left', 'right') },
|
||||
'audio.src': { noDefault: true, arb: urlArb, degen: '' },
|
||||
'youtube.src': { noDefault: true, arb: urlArb },
|
||||
// #351 (this PR): youtube width/height/align ride in the `<!--youtube {…}-->`
|
||||
// comment JSON (String()-coerced dimensions, non-center align only).
|
||||
'youtube.width': { arb: widthStr },
|
||||
'youtube.height': { arb: widthStr },
|
||||
'youtube.align': { arb: str('left', 'right') },
|
||||
'pdf.src': { noDefault: true, arb: urlArb },
|
||||
'pdf.name': { arb: phraseArb },
|
||||
// #351 (this PR): pdf size/width/height ride in the `<!--pdf {…}-->` comment
|
||||
// (String()-coerced dimensions, read back as strings).
|
||||
'pdf.size': { arb: sizeStr },
|
||||
'pdf.width': { arb: widthStr },
|
||||
'pdf.height': { arb: widthStr },
|
||||
'drawio.src': { noDefault: true, arb: urlArb },
|
||||
// #351 (this PR): drawio family attrs ride in the `<!--drawio {…}-->` comment.
|
||||
// Dimensions/size/aspectRatio are String()-coerced numeric strings; title/alt
|
||||
// are text carried verbatim; align fuzzed non-center only.
|
||||
'drawio.width': { arb: widthStr },
|
||||
'drawio.height': { arb: widthStr },
|
||||
'drawio.size': { arb: sizeStr },
|
||||
'drawio.aspectRatio': { arb: aspectRatioStr },
|
||||
'drawio.align': { arb: str('left', 'right') },
|
||||
'drawio.title': { arb: letterPhraseArb },
|
||||
'drawio.alt': { arb: letterPhraseArb },
|
||||
'excalidraw.src': { noDefault: true, arb: urlArb },
|
||||
// #351 (this PR): excalidraw family attrs ride in the `<!--excalidraw {…}-->`
|
||||
// comment (same shape as the drawio family above).
|
||||
'excalidraw.width': { arb: widthStr },
|
||||
'excalidraw.height': { arb: widthStr },
|
||||
'excalidraw.size': { arb: sizeStr },
|
||||
'excalidraw.aspectRatio': { arb: aspectRatioStr },
|
||||
'excalidraw.align': { arb: str('left', 'right') },
|
||||
'excalidraw.title': { arb: letterPhraseArb },
|
||||
'excalidraw.alt': { arb: letterPhraseArb },
|
||||
'attachment.url': { noDefault: true, arb: urlArb },
|
||||
'attachment.name': { arb: phraseArb },
|
||||
// ── callout / status ─────────────────────────────────────────────────────
|
||||
@@ -194,14 +247,26 @@ const OVERRIDES: Record<string, AttrPolicy> = {
|
||||
// widthMode round-trips via the `data-width-mode` attribute (verified P1+P2),
|
||||
// so it is fuzzed, not frozen.
|
||||
'columns.widthMode': { always: true, arb: str('custom') },
|
||||
// PINNED-BUG: parseFloat import drops the `%` unit -> P2 churn. See
|
||||
// counterexamples.test.ts (columns-column-width-percent.json).
|
||||
'column.width': { frozen: true },
|
||||
// column.width is a unitless flex-grow NUMBER (matches editor-ext column.ts);
|
||||
// parseHTML does parseFloat, so String(50) === "50" both ways and a numeric
|
||||
// width round-trips byte-stably. Value-fuzzed as a number.
|
||||
'column.width': { arb: num(25, 50, 75) },
|
||||
// ── embed (schema keeps width/height NUMERIC, not string-coerced) ─────────
|
||||
'embed.src': { noDefault: true, arb: urlArb, degen: '' },
|
||||
'embed.provider': { noDefault: true, arb: str('iframe', 'youtube', 'vimeo') },
|
||||
'embed.width': { always: true, frozen: true },
|
||||
'embed.height': { always: true, frozen: true },
|
||||
// #351 (this PR): the embed schema defaults width/height to the NUMBERS 800/600
|
||||
// and the converter only emits them when they differ. But the value round-trips
|
||||
// as a STRING: export stringifies into the comment JSON (String(width)) and the
|
||||
// import path (embedToHtml -> data-width -> embed parseHTML) reads it back as a
|
||||
// string, so an authored NUMBER 400 would diverge under P1 (400 vs "400"). Fuzz
|
||||
// as numeric STRINGS avoiding "800"/"600" so they round-trip byte-stably. The
|
||||
// 800/600 numeric default state still round-trips (omitted on export, re-
|
||||
// materialized as the numeric default). `always` stays because these are
|
||||
// materialized on import but absent from canonicalize's KNOWN_DEFAULTS.
|
||||
'embed.width': { always: true, arb: str('400', '1000', '1200') },
|
||||
'embed.height': { always: true, arb: str('300', '500', '900') },
|
||||
// align default "center" is dropped on export; fuzz non-center only.
|
||||
'embed.align': { arb: str('left', 'right') },
|
||||
// ── subpages / math / htmlEmbed ──────────────────────────────────────────
|
||||
'subpages.recursive': { always: true, arb: fc.constant(true) }, // boolean, default false
|
||||
'mathBlock.text': { noDefault: true, arb: str('x^2', 'a < b', '\\frac{1}{2}'), degen: '' },
|
||||
|
||||
@@ -7,18 +7,17 @@ import { markdownToProseMirror } from '../../src/lib/markdown-to-prosemirror.js'
|
||||
import { docsCanonicallyEqual } from '../../src/lib/canonicalize.js';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #351 committed counterexamples — REAL round-trip bugs surfaced by the flat
|
||||
// generative probing (attribute level). Each is pinned here as an `it.fails`
|
||||
// (vitest passes ONLY WHILE the assertion still fails), so that the day the
|
||||
// underlying src/ bug is fixed, the `it.fails` starts PASSING and vitest turns
|
||||
// this test RED — forcing us to delete the counterexample and (per the epic
|
||||
// guardrail) tighten the generator. A bare `it.fails` would ship silent
|
||||
// corruption, so every case below carries a loud `// BUG #351:` explanation.
|
||||
// #351 committed counterexample — a REAL round-trip bug surfaced by the flat
|
||||
// generative probing (attribute level). The bug below is now FIXED in src/
|
||||
// (maintainer-approved), so this case is a permanent PASSING regression pin:
|
||||
// the exact document that once lost data now round-trips cleanly, and the
|
||||
// fixture stays in test/fixtures/counterexamples/ forever (per the epic
|
||||
// guardrail) to guard against a re-regression. The case carries a loud
|
||||
// `// BUG #351 (FIXED):` note recording the defect and its fix site.
|
||||
//
|
||||
// These bugs are NOT worked around by weakening any property: the offending
|
||||
// attribute is kept OUT of the P1/P2 generators (documented in
|
||||
// attr-arbitraries.ts), and the exact failing document lives here as the
|
||||
// regression pin. FIXING the bug is a separate, maintainer-approved src/ change.
|
||||
// With the bug fixed, the offending attribute is now VALUE-FUZZED by the
|
||||
// generator (see attr-arbitraries.ts: orderedList.start), not held out — this
|
||||
// committed document is the minimal, human-readable pin.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const here = path.dirname(fileURLToPath(import.meta.url));
|
||||
@@ -28,47 +27,20 @@ function loadDoc(file: string): any {
|
||||
return JSON.parse(readFileSync(path.join(fixtureDir, file), 'utf8')).doc;
|
||||
}
|
||||
|
||||
describe('#351 counterexamples (known round-trip bugs, pinned as it.fails)', () => {
|
||||
// BUG #351: a `column` with a PERCENTAGE width ("50%") is not byte-stable.
|
||||
// The column schema parses `data-width` with parseFloat, dropping the '%':
|
||||
// md1 = '...data-width="50%"...' (first export)
|
||||
// re-import stores width = 50 (number)
|
||||
// md2 = '...data-width="50"...' (second export) => md2 !== md1
|
||||
// A permanent GS-EDIT-REVERT churn on every git-sync pull. The editor stores
|
||||
// column widths as percentages, so this is a genuine defect. The fix is in
|
||||
// src/lib/docmost-schema.ts (column.width parseHTML must preserve the unit)
|
||||
// and is out of scope for this test-only PR.
|
||||
it.fails('column percentage width is byte-stable (P2)', async () => {
|
||||
const doc = loadDoc('columns-column-width-percent.json');
|
||||
const md1 = convertProseMirrorToMarkdown(doc);
|
||||
const doc2 = await markdownToProseMirror(md1);
|
||||
const md2 = convertProseMirrorToMarkdown(doc2);
|
||||
// This assertion currently FAILS (md2 drops the '%'), which is exactly what
|
||||
// `it.fails` expects. When the schema is fixed, it will PASS and flip this
|
||||
// test red — our cue to remove the pin.
|
||||
expect(md2).toBe(md1);
|
||||
});
|
||||
|
||||
// BUG #351: an `orderedList` with a non-1 `start` loses its start number.
|
||||
// CommonMark CAN express this ("5." starts the list at 5), but the converter
|
||||
// always emits "1." and ignores `attrs.start` (markdown-converter.ts renders
|
||||
// `${index + 1}.`; the <ol> HTML path also omits `start`):
|
||||
describe('#351 counterexamples (round-trip bugs, now FIXED — permanent regression pins)', () => {
|
||||
// BUG #351 (FIXED): an `orderedList` with a non-1 `start` lost its start
|
||||
// number. CommonMark CAN express this ("5." starts the list at 5), but the
|
||||
// converter always emitted "1." and ignored `attrs.start`:
|
||||
// doc.start = 5 -> md1 = "1. alpha" (start dropped on export)
|
||||
// re-import stores start = 1 => docsCanonicallyEqual(rt, doc) === false
|
||||
// This is a P1 (semantic round-trip) loss of the SAME class as column.width:
|
||||
// representable in markdown, silently dropped by the converter. It is pinned
|
||||
// here as the LOUD counterexample rather than being masked as an "accepted
|
||||
// normalization" in the generator — per the epic guardrail, deciding
|
||||
// accept-vs-fix for a markdown-representable loss is a MAINTAINER call, so this
|
||||
// stays a visible known-bug until the maintainer rules on it. The fix would be
|
||||
// in src/lib/markdown-converter.ts (emit the start number on the first item)
|
||||
// and is out of scope for this test-only PR.
|
||||
it.fails('ordered list start number is preserved (P1)', async () => {
|
||||
// re-import stored start = 1 => docsCanonicallyEqual(rt, doc) === false
|
||||
// A P1 (semantic round-trip) loss of the SAME class as column.width. FIXED in
|
||||
// this PR in src/lib/markdown-converter.ts (the orderedList markdown path emits
|
||||
// `${start + index}.`; the raw-HTML path emits `<ol start="N">`). tiptap's
|
||||
// StarterKit / marked parse the start back, so it round-trips. Permanent P1 pin.
|
||||
it('ordered list start number is preserved (P1)', async () => {
|
||||
const doc = loadDoc('ordered-list-start.json');
|
||||
const md1 = convertProseMirrorToMarkdown(doc);
|
||||
const doc2 = await markdownToProseMirror(md1);
|
||||
// Currently FAILS: doc2.start === 1 while doc.start === 5. When the converter
|
||||
// preserves `start`, this PASSES and flips the test red — remove the pin then.
|
||||
expect(docsCanonicallyEqual(doc2, doc)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { envInt } from './env-int.js';
|
||||
|
||||
// Unit coverage for the shared PROPERTY_SEED / PROPERTY_NUM_RUNS parser. The key
|
||||
// contract is that an explicit "0" is honored (a valid fast-check seed) while
|
||||
// empty/absent/non-numeric fall back to the default.
|
||||
describe('envInt', () => {
|
||||
it('honors an explicit "0" (does not fall back)', () => {
|
||||
expect(envInt('0', 42)).toBe(0);
|
||||
});
|
||||
it('falls back on empty string', () => {
|
||||
expect(envInt('', 42)).toBe(42);
|
||||
});
|
||||
it('falls back on undefined', () => {
|
||||
expect(envInt(undefined, 42)).toBe(42);
|
||||
});
|
||||
it('falls back on a non-numeric string', () => {
|
||||
expect(envInt('abc', 42)).toBe(42);
|
||||
});
|
||||
it('parses a plain integer string', () => {
|
||||
expect(envInt('300', 42)).toBe(300);
|
||||
});
|
||||
it('parses a negative integer', () => {
|
||||
expect(envInt('-5', 42)).toBe(-5);
|
||||
});
|
||||
it('parses a large integer', () => {
|
||||
expect(envInt('1073741824', 42)).toBe(1073741824);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,10 @@
|
||||
/**
|
||||
* Parse an integer-ish environment variable with a default fallback.
|
||||
*
|
||||
* Used to read PROPERTY_SEED / PROPERTY_NUM_RUNS in the generative property
|
||||
* suites. An unset/empty/non-numeric value falls back to `dflt`, but an explicit
|
||||
* `"0"` is honored (a valid fast-check seed) — `Number(x) || dflt` would wrongly
|
||||
* swallow 0. See flat-roundtrip.property.test.ts / nested-roundtrip.property.test.ts.
|
||||
*/
|
||||
export const envInt = (v: string | undefined, dflt: number): number =>
|
||||
v !== undefined && v !== '' && Number.isFinite(Number(v)) ? Number(v) : dflt;
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
coveredTypes,
|
||||
KNOWN_UNCOVERED,
|
||||
} from './node-generators.js';
|
||||
import { envInt } from './env-int.js';
|
||||
|
||||
// ── Attribute-value coverage allowlist ──────────────────────────────────────
|
||||
// The node/mark completeness contract guarantees every TYPE is generated, but
|
||||
@@ -28,34 +29,32 @@ import {
|
||||
// a NEW attribute (or a newly-frozen one) that lands in this bucket flips the
|
||||
// snapshot test red and forces a reviewer to classify it. Each belongs to one of:
|
||||
// - internal/opaque ids & placeholders (attachmentId, slugId, placeholder,
|
||||
// creatorId, anchorId) — no meaningful non-default to assert;
|
||||
// - dimensions/among the media family with no standalone md form here
|
||||
// (aspectRatio, size, caption, drawio/excalidraw/pdf/video/youtube w/h/align)
|
||||
// — round-trip candidates deferred to a later PR, not silently dropped;
|
||||
// creatorId, anchorId, mime) — no meaningful non-default to assert. These stay
|
||||
// frozen: their value is an opaque token carried verbatim, not a round-trip
|
||||
// shape worth fuzzing;
|
||||
// - ACCEPTED limitations with no md representation (indent, callout.icon,
|
||||
// orderedList.type, table spans/bg/colwidth);
|
||||
// - PINNED bugs (column.width, orderedList.start) tracked in
|
||||
// counterexamples.test.ts.
|
||||
// orderedList.type, table spans/bg/colwidth).
|
||||
// The media dimension/family attrs (image/video/youtube/drawio/excalidraw/pdf/
|
||||
// embed width/height/align/size/aspectRatio/caption/title/alt) that were
|
||||
// previously "deferred to a later PR" are IMPLEMENTED (value-fuzzed) in THIS PR
|
||||
// via the OVERRIDES table in attr-arbitraries.ts — they ride in the discriminator
|
||||
// comment JSON and round-trip byte-stably, so they are no longer allowlisted.
|
||||
const ATTR_VALUE_FUZZ_ALLOWLIST = new Set<string>([
|
||||
'attachment.attachmentId', 'attachment.mime', 'attachment.placeholder', 'attachment.size',
|
||||
'audio.attachmentId', 'audio.placeholder', 'audio.size',
|
||||
'callout.icon', 'column.width',
|
||||
'drawio.align', 'drawio.alt', 'drawio.aspectRatio', 'drawio.attachmentId',
|
||||
'drawio.height', 'drawio.size', 'drawio.title', 'drawio.width',
|
||||
'embed.align', 'embed.height', 'embed.width',
|
||||
'excalidraw.align', 'excalidraw.alt', 'excalidraw.aspectRatio', 'excalidraw.attachmentId',
|
||||
'excalidraw.height', 'excalidraw.size', 'excalidraw.title', 'excalidraw.width',
|
||||
'callout.icon',
|
||||
'drawio.attachmentId',
|
||||
'excalidraw.attachmentId',
|
||||
'heading.indent',
|
||||
'image.aspectRatio', 'image.attachmentId', 'image.caption', 'image.placeholder', 'image.size',
|
||||
'image.attachmentId', 'image.placeholder',
|
||||
'mention.anchorId', 'mention.creatorId', 'mention.slugId',
|
||||
'orderedList.start', 'orderedList.type', 'paragraph.indent',
|
||||
'pdf.attachmentId', 'pdf.height', 'pdf.placeholder', 'pdf.size', 'pdf.width',
|
||||
'orderedList.type', 'paragraph.indent',
|
||||
'pdf.attachmentId', 'pdf.placeholder',
|
||||
'tableCell.backgroundColor', 'tableCell.backgroundColorName', 'tableCell.colspan',
|
||||
'tableCell.colwidth', 'tableCell.rowspan',
|
||||
'tableHeader.backgroundColor', 'tableHeader.backgroundColorName', 'tableHeader.colspan',
|
||||
'tableHeader.colwidth', 'tableHeader.rowspan',
|
||||
'video.align', 'video.aspectRatio', 'video.attachmentId', 'video.placeholder', 'video.size',
|
||||
'youtube.align', 'youtube.height', 'youtube.width',
|
||||
'video.attachmentId', 'video.placeholder',
|
||||
]);
|
||||
|
||||
// ── MARK attribute-value coverage ───────────────────────────────────────────
|
||||
@@ -113,13 +112,20 @@ 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.
|
||||
// The parser is shared with the nested suite (env-int.ts) and unit-tested there.
|
||||
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');
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
} from '../../src/lib/index.js';
|
||||
import { firstDivergence } from '../roundtrip-helpers.js';
|
||||
import { schema, docArb } from './doc-generator.js';
|
||||
import { envInt } from './env-int.js';
|
||||
|
||||
// Each run does a real convert + jsdom parse; give ample headroom so the suite
|
||||
// is deterministic under parallel worker load (matching the flat sibling suite).
|
||||
@@ -33,12 +34,19 @@ 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.
|
||||
// The parser is shared with the flat suite (env-int.ts) and unit-tested there.
|
||||
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<any> => markdownToProseMirror(md);
|
||||
|
||||
@@ -217,8 +217,9 @@ const colChildOf = (doc2: any) =>
|
||||
doc2?.content?.[0]?.content?.[0]?.content?.[0];
|
||||
|
||||
describe('converter gap coverage — emission branches (specs 1–11)', () => {
|
||||
// 1. orderedList renders index+1 and DROPS the start attribute.
|
||||
it('orderedList start:5 restarts numbering at 1 (start attr ignored)', () => {
|
||||
// 1. orderedList honors the start attribute (FIXED #351): markers count up
|
||||
// from `start` ("5.","6.",…), which CommonMark round-trips.
|
||||
it('orderedList start:5 numbers from 5 (start attr honored)', () => {
|
||||
const out = convertProseMirrorToMarkdown(
|
||||
doc({
|
||||
type: 'orderedList',
|
||||
@@ -229,7 +230,7 @@ describe('converter gap coverage — emission branches (specs 1–11)', () => {
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(out).toBe('1. a\n2. b');
|
||||
expect(out).toBe('5. a\n6. b');
|
||||
});
|
||||
|
||||
// 2. An empty paragraph contributes an empty segment between two "\n\n" joins.
|
||||
|
||||
@@ -365,7 +365,7 @@ describe('media / attachment / container full-attribute golden coverage', () =>
|
||||
);
|
||||
});
|
||||
|
||||
it('orderedList inside a column renders via blockToHtml as <ol> (start attr DROPPED) with bold->strong, code->code', () => {
|
||||
it('orderedList inside a column renders via blockToHtml as <ol start="N"> (start attr PRESERVED) with bold->strong, code->code', () => {
|
||||
const out = c({
|
||||
type: 'columns',
|
||||
attrs: { layout: 'two' },
|
||||
@@ -391,13 +391,13 @@ describe('media / attachment / container full-attribute golden coverage', () =>
|
||||
},
|
||||
],
|
||||
});
|
||||
// blockToHtml orderedList path emits a plain <ol> with no start attribute,
|
||||
// and inlineToHtml maps bold->strong, code->code.
|
||||
// blockToHtml orderedList path emits <ol start="3"> (FIXED #351), and
|
||||
// inlineToHtml maps bold->strong, code->code.
|
||||
expect(out).toContain(
|
||||
'<ol><li><p><strong>a</strong></p></li><li><p><code>b</code></p></li></ol>',
|
||||
'<ol start="3"><li><p><strong>a</strong></p></li><li><p><code>b</code></p></li></ol>',
|
||||
);
|
||||
// The start:3 attr is NOT preserved in the HTML/column container path.
|
||||
expect(out).not.toContain('start=');
|
||||
// The start:3 attr IS preserved in the HTML/column container path.
|
||||
expect(out).toContain('start="3"');
|
||||
});
|
||||
|
||||
it('hardBreak inside a column renders as <br> via inlineToHtml (not the markdown two-space form)', () => {
|
||||
|
||||
@@ -178,12 +178,12 @@ describe('blockToHtml: heading / codeBlock(lang & no-lang) / bulletList inside c
|
||||
// A colspan>1 cell forces the WHOLE table to the raw-<table> HTML fallback
|
||||
// (markdown-converter.ts ~287-331). renderHtmlCell emits colspan + align attrs
|
||||
// (312-316) and renders each block child via blockToHtml. An orderedList child
|
||||
// hits the blockToHtml orderedList branch (726-729), which emits
|
||||
// <ol><li><p>..</p></li>..</ol> — the schema's `start` attr is NOT emitted by
|
||||
// this HTML <ol> branch.
|
||||
// hits the blockToHtml orderedList branch, which emits
|
||||
// <ol start="N"><li><p>..</p></li>..</ol> — the schema's non-1 `start` attr IS
|
||||
// emitted by this HTML <ol> branch (FIXED #351).
|
||||
// ---------------------------------------------------------------------------
|
||||
describe('spanned table: renderHtmlCell colspan/align + orderedList block child', () => {
|
||||
it('renders the colspan/align cell with an <ol> (start attr is dropped)', () => {
|
||||
it('renders the colspan/align cell with an <ol start="N"> (start attr preserved)', () => {
|
||||
const out = convertProseMirrorToMarkdown(
|
||||
doc({
|
||||
type: 'table',
|
||||
@@ -213,11 +213,11 @@ describe('spanned table: renderHtmlCell colspan/align + orderedList block child'
|
||||
expect(out).toBe(
|
||||
'<table><tbody><tr>' +
|
||||
'<td colspan="2" align="center">' +
|
||||
'<ol><li><p>one</p></li><li><p>two</p></li></ol>' +
|
||||
'<ol start="3"><li><p>one</p></li><li><p>two</p></li></ol>' +
|
||||
'</td>' +
|
||||
'</tr></tbody></table>',
|
||||
);
|
||||
// The HTML <ol> branch does not propagate the ProseMirror `start` attribute.
|
||||
expect(out).not.toContain('start');
|
||||
// The HTML <ol> branch propagates the ProseMirror `start` attribute.
|
||||
expect(out).toContain('start="3"');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js';
|
||||
import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #351 regression — DEGENERATE orderedList `start` normalization (DO-1).
|
||||
//
|
||||
// Only an INTEGER >= 2 is a valid explicit start. A degenerate start (0,
|
||||
// negative, fractional, non-number) MUST collapse to the default 1 on export:
|
||||
// emitting the raw value would produce an untokenizable marker (e.g. "-3.",
|
||||
// "2.5.") that `marked` cannot parse, causing the WHOLE orderedList (and its
|
||||
// listItems) to re-import as a PARAGRAPH — structural corruption. This test
|
||||
// pins that each degenerate start:
|
||||
// (1) exports to "1."/"2." markers (default numbering),
|
||||
// (2) re-imports as a VALID `orderedList` (not a paragraph) with `start`
|
||||
// defaulting, structure preserved,
|
||||
// (3) is byte-stable (md1 === md2).
|
||||
//
|
||||
// Mutation check: revert the converter guard to `Number(raw) || 1` and this
|
||||
// test goes RED (a fractional/negative start leaks into the marker).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const makeDoc = (start: unknown) => ({
|
||||
type: 'doc',
|
||||
content: [
|
||||
{
|
||||
type: 'orderedList',
|
||||
attrs: { type: null, start },
|
||||
content: [
|
||||
{
|
||||
type: 'listItem',
|
||||
content: [
|
||||
{ type: 'paragraph', content: [{ type: 'text', text: 'alpha' }] },
|
||||
],
|
||||
},
|
||||
{
|
||||
type: 'listItem',
|
||||
content: [
|
||||
{ type: 'paragraph', content: [{ type: 'text', text: 'beta' }] },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
describe('#351 orderedList degenerate start normalization', () => {
|
||||
for (const start of [0, -3, 2.5]) {
|
||||
it(`start=${start} exports as default "1."/"2." markers`, () => {
|
||||
const md = convertProseMirrorToMarkdown(makeDoc(start));
|
||||
expect(md).toContain('1. alpha');
|
||||
expect(md).toContain('2. beta');
|
||||
// The degenerate value must NOT leak into any marker.
|
||||
expect(md).not.toMatch(/-?\d*\.\d+\.\s/); // no fractional marker like "2.5."
|
||||
expect(md).not.toContain('-3.');
|
||||
});
|
||||
|
||||
it(`start=${start} re-imports as a valid orderedList (not a paragraph)`, async () => {
|
||||
const md = convertProseMirrorToMarkdown(makeDoc(start));
|
||||
const doc = (await markdownToProseMirror(md)) as any;
|
||||
const top = doc.content?.[0];
|
||||
expect(top?.type).toBe('orderedList');
|
||||
// start defaults (never the degenerate value); tiptap materializes 1.
|
||||
expect(top?.attrs?.start ?? 1).toBe(1);
|
||||
expect(top?.content?.length).toBe(2);
|
||||
});
|
||||
|
||||
it(`start=${start} is byte-stable across a re-export`, async () => {
|
||||
const md1 = convertProseMirrorToMarkdown(makeDoc(start));
|
||||
const doc = await markdownToProseMirror(md1);
|
||||
const md2 = convertProseMirrorToMarkdown(doc);
|
||||
expect(md2).toBe(md1);
|
||||
});
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user