Skip to content

fix(spec): the generated upgrade guide keeps a surface in one code span when it carries its own backticks - #27724

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-2ul8sb
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-2ul8sb

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by 田騏 · project thread

Fixes #27532

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:协议升级指南生成器把每个条目的 surface 按 CommonMark 规则包进一个代码段(围栏比内部最长反引号串多一个),转换表单元格另外转义 |;两处渲染点共用一个入口,516 条条目一条未手改。
  • 为什么改:旧写法用单反引号包 surface,而 183 条语义条目在 surface 里自带反引号,渲染出来是代码与正文交替的碎片;两条带 | 的转换条目会把表格行撕成 6 格。
  • 风险与回滚:只动生成脚本与生成产物(随包发布的 protocol-upgrade-guide.md 与文档站页面),不动 schema、取值、错误码;不含反引号的 surface 字节不变。回滚 = revert 本 PR。
  • 先看:packages/spec/scripts/lib/markdown-code-span.ts(围栏与补空格规则)→ packages/spec/scripts/build-upgrade-guide.ts 的 tableRow / surfaceSpan 两处调用 → packages/spec/scripts/upgrade-guide-surface-span.test.ts 的真实条目集钉子。
  • 两分钟验证:pnpm --filter @objectstack/spec exec vitest run scripts/upgrade-guide-surface-span.test.ts,预期 15 个测试全绿;把任一渲染点改回单反引号再跑,预期变红。
  • 需要你做的:无,席位落地(等契约复审)。

Fable review: not run — the account's Fable quota is exhausted; the PR waits for the contract review (maintainer, 2026-10-29).

What changes

  • packages/spec/scripts/lib/markdown-code-span.ts (new): codeSpan(text) fences a one-line text with a backtick string one longer than the longest backtick run inside it, and pads one space on each side when the text starts or ends with a backtick or a space. That is CommonMark §6.1's own rule for code that contains backticks. A text with no backtick and no edge space comes out byte-identical to the old single-backtick span. A blank or multi-line text is refused, and the refusal names the entry. readCodeSpan is the reading half of the same rule; only the pin uses it.
  • packages/spec/scripts/build-upgrade-guide.ts: both render sites (the conversion table row and the semantic list item) go through one surfaceSpan(id, surface) door. The conversion row is built by tableRow, which escapes | in every cell. GFM ends a cell at an unescaped pipe even inside a code span, and two live conversion surfaces carry one (navigation-mode-overlay-values-removed, form-view-presentation-types-removed).
  • Why a longer fence and not prose: a surface is a key path or an expression, and it reads as code. Rendering it as prose would make the guide's two forms of one surface differ from what migrate meta prints. A longer fence keeps one code span per surface and shows the author's backticks literally. No entry is edited.
  • 18.visibility-predicate-has-argument-refused.ts: the workaround note ("no backticks in surface") is dropped. Its surface is untouched.
  • export-job-family-retirement.test.ts, package-upgrade-family-retirement.test.ts: each drops one expect(entry!.surface).not.toMatch(/BACKTICK/) line. Those lines forbade backticks in a surface only because of this defect. Every other assertion in both tests stands.
  • .changeset/27532-upgrade-guide-surface-span.md: patch on @objectstack/spec. protocol-upgrade-guide.md is in the package's files[] and is generated at publish, so the shipped bytes move.
  • Committed outputs regenerated: none exist. packages/spec/protocol-upgrade-guide.md and content/docs/protocol-upgrade/ are both gitignored and generated at publish or docs build. docs/protocol-upgrade-guide.md is a pointer stub that nothing generates. git ls-files lists no rendered guide.

Tests

packages/spec/scripts/upgrade-guide-surface-span.test.ts (15 tests) has three layers:

  • The helper: fence length, padding, refusals, and a round trip through the reader. A lit control shows the OLD single-backtick wrapping of every real marked surface reads as broken, so the reader can see a break.
  • The real entry set: every semantic surface (516) and every conversion surface (109) round-trips through codeSpan/readCodeSpan. A new entry cannot bring the defect back, whatever backticks it carries.
  • The generated guide: the REAL generator runs (tsx build-upgrade-guide.ts --out). Its bytes are read back span by span: each semantic line yields its surface verbatim followed by →. Each conversion row is exactly four GFM cells, and the surface is one span filling its cell. A second lit control: the old row shape of a pipe-carrying surface splits into more than four cells, on a literal row and on every real pipe-carrying conversion.

Ablation on head 34de285ecf, each leg through scripts/ablation-replace.mjs, with anchor hit, blob change and restore all verified on disk. There is no dist/ leg: the test imports ../src and spawns the script source by relative path.

  • Semantic site reverted to `${s.surface}`: 1 failed / 14 passed (action-descriptor-is-async-retired first). Restored, blob == HEAD.
  • Conversion site reverted to main's row: 1 failed / 14 passed (navigation-mode-overlay-values-removed: 6 cells, not 4). Restored, blob == HEAD.
  • Helper fence forced to one backtick: 5 failed / 10 passed. Restored, blob == HEAD.

Acceptance notes

  • Measured size of the defect: 183 of the 516 semantic surfaces carry a backtick, and all 183 broke under the old wrapping (markdown-it, rendered in memory). The card's "most" overstates it, but the fix and the pin are the same either way.
  • Two independent renderers were checked against the fixed guide, beyond the pin's own CommonMark reader. remark with GFM (micromark, the docs site's parser) gave 516/516 semantic spans and 104/104 rendered conversion rows verbatim. markdown-it gave 516/516 semantic spans. An escaped pipe inside a table-cell code span renders as | in remark, markdown-it and marked alike.
  • summary and the load-window cell are now pipe-escaped too, because tableRow escapes every cell. No current summary carries a pipe, so no byte moves there today.

Verification

All of it was run on 34de285ecf: the branch merged with origin/main a75ddba178, with a clean merge and no generated-file conflict. The branch delta vs main is unchanged at 7 files, +356 / −8.

spec tests and typecheck
  • pnpm --filter @objectstack/spec test: Test Files 754 passed (754), Tests 22406 passed, 1 todo; exit 0.
  • pnpm --filter @objectstack/spec typecheck: exit 0. tsconfig.scripts.json --listFiles compiles all three touched script files.
  • Targeted run (pin, docs-page tests and the two edited sibling tests): Test Files 4 passed, Tests 47 passed.
derived gates (dispatch-gates --commands, 86 commands)
  • 85 exit 0. That includes check:upgrade-guide, check:adr-0087-registration --base origin/main, check:changeset-no-major, check:empty-changeset, check:spec-changes, check:docs, check:api-surface, check:nul-bytes, check:cross-package-test-inputs and check:pm-dispatch-gates.
  • check:doc-formula-expressions and check:lean-entry-closure first exited 3 (PREREQUISITE NOT MET, no dist/). Both exit 0 after building their closures.
  • check:dual-build-cjs-loads: NOT MEASURED. It exits 3 because about 93 packages have no dist/ in this worktree. The diff moves no package's built output; CI's Lint & Repo Gates runs it after the build.
  • --ran reconciliation: 86 derived, 85 run, 1 NOT-MEASURED (derived from the recorded exit 3), 0 UNRUN.

🤖 Generated with Claude Code

…an when it carries its own backticks

The protocol upgrade guide wrapped every conversion's and semantic entry's
`surface` in a single-backtick code span. 183 of the 516 semantic entries mark
their own inline code inside the surface, and CommonMark closes a
single-backtick span at the first backtick inside it, so those lines rendered
as alternating code and prose fragments. Two conversion surfaces carry a `|`,
which split their table row into extra cells.

Both render sites now go through one helper, `codeSpan` in
`scripts/lib/markdown-code-span.ts`: the span is fenced one backtick longer
than the longest run inside the text (CommonMark's rule), padded when the text
starts or ends with a backtick, and byte-identical for a backtick-free text.
A table row escapes `|` in every cell. `readCodeSpan`, the reading half of the
same rule, lets the pin decode what the generator wrote.

The pin (`scripts/upgrade-guide-surface-span.test.ts`) covers the helper, the
real entry set (every surface round-trips), and the generated guide read back
span by span, so a new entry cannot bring the defect back. The workaround note
in `18.visibility-predicate-has-argument-refused.ts` is dropped.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2gP8YEs7CoWrbVsSAi
…bling tests stop forbidding backticks in a surface

Applies the independent review's findings on the previous head.

- The generated-guide pin splits each conversion row into cells as GFM does
  (every pipe ends a cell, inside a code span or not, except an escaped one)
  and expects exactly four, with the surface span filling its own cell; its
  lit control shows the old single-backtick row of a pipe-carrying surface
  splitting into extra cells. Before this the pin could not see the pipe
  defect the row escape fixes.
- Two sibling tests pinned the workaround the card drops ("No backticks in
  surface: the upgrade guide renders it inside a code span") on real entries;
  the sentence is now false and the assertion forbade what is legal, so both
  pairs go.
- The generator's refusal names the entry; the helper's docblocks say
  "no backtick and no leading or trailing space" instead of "backtick-free",
  and give the table-row reason for refusing a line ending; the reader treats
  a lone carriage return as a line ending and strips padding only around
  spaces; the conversion entry-set pin asserts no surface carries a backslash,
  where GFM renderers part ways on an escaped pipe.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2gP8YEs7CoWrbVsSAi
… pipe-carrying conversions

Applies the delta review's findings. The lit control for the table-row escape
starts from a literal old-style row with a pipe in its surface, so it keeps
biting after the two retirement kits that carry a pipe today age out one
major on; the real-registry loop follows with no count floor. The count floors
on the other real-set pins drop to "more than zero", the liveness they need.
The conversion-surface pin forbids an escaped pipe, the one pair GFM renderers
read two ways, instead of any backslash.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2gP8YEs7CoWrbVsSAi
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/spec/src/migrations/entries/semantic/18.visibility-predicate-has-argument-refused.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/src/migrations/entries/semantic/18.visibility-predicate-has-argument-refused.ts) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 167 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 86c9d536b413d871d8acb71ede406f4a0cdf1889 → packageMentionDocs.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:spec#4 · marchtian · session_017ihL2fi9zfJhUTubyJYb6V

席位核对 · ACCEPT (lane part) · card #27532 · 2026-10-29T20:52Z

  • Shape: 7 files, +356 / −8. Both render sites in packages/spec/scripts/build-upgrade-guide.ts go through one surfaceSpan → codeSpan helper (packages/spec/scripts/lib/markdown-code-span.ts): a fence one backtick longer than the longest run inside, CommonMark §6.1; a conversion-table cell escapes |. A pin over the real registries (516 semantic, 109 conversion surfaces) reads the generated guide back; the workaround note in 18.visibility-predicate-has-argument-refused.ts is dropped; two sibling tests stop forbidding backticks in a surface. @objectstack/spec patch changeset (protocol-upgrade-guide.md ships in the package). Matches triage 6496306402: the generator is fixed once, no entry hand-edited.
  • Dev: finished on opus under the quota-exhaustion exemption (claim amendment on [finding] the generated upgrade guide wraps each semantic entry's surface in a single-backtick code span, so a surface that carries its own backticks (most of the ~516 entries) renders broken #27532). Ablation: all three legs red without the fix. os-dev-report 6508030760.
  • Owed before any landing stroke: the CONTRACT_REVIEW_TIER review. reviewFace hits packages/spec/src/migrations/entries/semantic/18.visibility-predicate-has-argument-refused.ts (a published-schema-face path, comment-only hunk). This seat cannot serve that tier now, so it hands the review over below.
  • CI: in progress on this head; the landing also waits for every check to be green.

Contract-review-handoff: triage — CONTRACT_REVIEW_TIER unavailable (HTTP 429 "You've reached your Fable limit", 2026-10-29T17:05Z) · head 34de285

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-sales · session_01L6bhgV4fMGJHSdSdx3u9mK

Contract-review-claim: session_01L6bhgV4fMGJHSdSdx3u9mK · head 34de285 · 2026-10-29T21:17Z


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

triage#1 · os-sales · session_01L6bhgV4fMGJHSdSdx3u9mK

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 34de285ecf266bc596a06dd76a5478ce4d32deb6
Local-runs: none
Scope: packages/spec/src/migrations/entries/semantic/18.visibility-predicate-has-argument-refused.ts + packages/spec/scripts/build-upgrade-guide.ts, packages/spec/scripts/lib/markdown-code-span.ts, packages/spec/scripts/lib/upgrade-guide-docs.ts, packages/spec/src/migrations/types.ts, packages/spec/src/api/export-job-family-retirement.test.ts, packages/spec/src/kernel/package-upgrade-family-retirement.test.ts, packages/spec/src/system/compliance-families-retirement.test.ts, packages/cli/src/commands/migrate/meta.ts, .changeset/27532-upgrade-guide-surface-span.md

Inputs read: card #27532 (body and all 5 comments: triage 6496306402, release 6500852824, claim 6501175459, claim amendment 6506624943, os-dev-report 6508030760), PR #27724 (body, 7-file list, net diff from the files endpoint, comments 6508021606 / 6508068835 / 6508551079) and the 53 check-runs on the head. The hits are read off record-recognisers.mjs --paths over the whole file list: the one semantic entry file; the other six paths are the lane seat's ACCEPT at TIER_DEFAULT (comment 6508068835) and are read here only as the code the hits hunk cites.

① Derived judgments

  • Accept set — unchanged, right. The hits hunk deletes a two-line comment; the entry's id, surface, replacement, reason and acceptanceCriteria are byte-identical to main. No schema, accepted value, tombstone, error code or served body moves anywhere in the diff.
  • The surface contract — the renderer-imposed "no backticks" convention is withdrawn, right. SemanticMigration.surface (types.ts:102) is documented as the dotted surface the change governs and never stated the backtick ban; the ban lived only in entry comments and three family tests, as a workaround for the single-backtick wrapping at build-upgrade-guide.ts:126 and :135 on main. At the head both render sites go through surfaceSpan → codeSpan, which fences by CommonMark 6.1 (one backtick longer than the longest run inside, padded when the text starts or ends with a backtick or a space), so the convention has no renderer left to serve.
  • Published artefact protocol-upgrade-guide.md and the docs pages — bytes move, right. Every semantic surface carrying a backtick (the PR measures 183 of 516) gains a longer fence; the two conversion rows whose surface carries a pipe (navigation-mode-overlay-values-removed, form-view-presentation-types-removed, read at conversions/registry.ts:6122 and :8951) gain an escaped pipe in their cell; every surface with no backtick and no edge space is byte-identical (codeSpan returns the plain single-backtick span for that case). lib/upgrade-guide-docs.ts is untouched and consumes hopSection's lines through HopInput.section, so the docs site pages (content/docs/protocol-upgrade/N.md) carry the same fix with no second render site — the card's "fix it once, in the renderer" holds in the tree.
  • objectstack migrate meta — unaffected, right. packages/cli/src/commands/migrate/meta.ts:790 and :800 print surface raw, not in a code span; the PR's argument for a longer fence over prose (the guide shows the same text migrate meta prints) is read as true.
  • New module scripts/lib/markdown-code-span.ts — no public surface, right. packages/spec/package.json files[] ships dist, src/**/*.zod.ts, the generated artefacts and protocol-upgrade-guide.md; scripts/ is not published, so codeSpan / readCodeSpan are build-time only.
  • The two edited family tests — right, but the sweep is one short. export-job-family-retirement.test.ts:214-215 and package-upgrade-family-retirement.test.ts:205-206 dropped expect(entry!.surface).not.toMatch(/BACKTICK/) whose stated reason was this defect; every other assertion stands. packages/spec/src/system/compliance-families-retirement.test.ts:260-262 at the head keeps the identical assertion with the identical reason ("the upgrade guide renders it inside a code span AND a table cell") — read as the one sibling the PR missed (④, should-fix).

② Semver level

  • patch on @objectstack/spec — right. protocol-upgrade-guide.md is in the package's files[] and is generated at publish by scripts/release-spec-changes.sh (the script's own docblock), so the shipped tarball's bytes move; a bug fix in a released package takes patch, and skip-changeset would be wrong here. No dist, type or JSON-schema output moves, so nothing higher is owed. Check Changeset is green on the head.
  • Clause-②: no — right, on the PR body and in the changeset body alike, with no arm: no accepted value, served body or error code moves; the only mover is the rendering of a generated document. The changeset also carries Metadata-protocol: none, which is right (no key, value or meaning an author writes changes), and no ! or BREAKING banner is owed. The changeset body names no model identifier.

③ Boundary flags

  • Check-runs on the head: 53, every one success or skipped (opt-in harnesses and the UI matrix shards), none failure. Check Changeset, Governed Surface Queue Guard, PR-body guards, Spec property liveness, Lint & Repo Gates, Test Core (8/8) and all four Type Check jobs green. These are the gate verdicts; nothing was re-run here.
  • Dev flag "independent review: not run" (PR body; os-dev-report deviation 1): answered — this record is the CONTRACT_REVIEW_TIER review the PR body says it waits for, rendered on the tree, not on the body's account. The _common.md the report names is a dispatch-order artefact, not a file in the tree; no further review is owed by this PR. Note for the seat: the body carries the word "Fable" twice in that line; the model-identifier rule (AGENTS.md, commit and body hygiene) reads a PR body as a carrier, the PR-body guards check passed on the head so the gate verdict stands, and the line is a restatement of the maintainer's order recorded on the card (claim amendment 6506624943) — not a must-fix, striking the model name before landing is the tidier form.
  • Deviation 2 (footer line form): PR-body guards green; cosmetic; answered.
  • Deviation 3 (commits 8e4d77a and d5876c7 written by the earlier card thread, the head is the merge commit): answered — the claim amendment moved the same seat to a subagent on the same branch, and Implemented-by below names that branch, which is the identity the implementation claim reads for mode:subagent.
  • open_questions: none declared, none found.
  • out_of_scope_findings (183 of 516, not "most"): confirmed as a report-only correction; and the card's premise "Only 18.visibility-predicate-… avoids this, with a comment" is wrong on main as well: 219 semantic entry files carry the same workaround comment (grep -rl "No backticks" packages/spec/src/migrations/entries/semantic). The card's remedy is unaffected by either miscount; the sweep in ④ is the carrier.
  • Docs Drift Check (comment 6508021606): the hits file yields no anchor. Read for the rule the diff carries: no page under content/docs, no ADR under docs/adr and no skill under .claude/skills teaches a "no backticks in surface" rule, so there is no documentation page this hunk falsifies.

④ Design judgment

packages/spec/src/migrations/entries/semantic/18.visibility-predicate-has-argument-refused.ts — the hunk removes the comment "No backticks in surface — build-upgrade-guide renders it inside a code span already, and a nested backtick would close it."

  • Against the code it cites (build-upgrade-guide.ts at the head): the comment's claim is false there. The semantic line is built as - **ID** — surfaceSpan(id, surface) → replacement, and surfaceSpan is codeSpan(surface, 'entry ID'), which fences with a backtick string one longer than the longest run inside the text; a nested backtick no longer closes the span. A comment that prescribes a workaround for a renderer that no longer needs it is a false statement in a governed file, so dropping it is the right change, and the card's third "Wanted" bullet asks for exactly this.
  • Against the scenario it is accepted against (ruling 6351260886 on Page/view visibleWhen publishes has(status) and has(record.items[0]) clean, then the console faults and fails open: adopting that check() verdict needs the maintainer's ruling (follow-up to #25267) #25281, ADR-0137 / ADR-0087 D3): the entry's surface, replacement, reason and acceptance criteria are byte-identical; the surface has no backtick and no leading or trailing space, so codeSpan returns the plain single-backtick span and this entry's rendered line in the guide and on the docs page does not move. migrate meta prints the surface raw. The ruling's prescription is untouched.
  • Tried to break it: (a) a later author puts backticks in this surface — fenced correctly, padded if a backtick sits at an edge, and the pin reads the generated bytes back to the author's text; (b) a pipe in a semantic surface — the semantic site is a list item, not a table row, and renders it literally, consistent with the raw CLI print; (c) a blank or multi-line surface — codeSpan refuses at generation and names the entry, where main rendered a broken line silently; (d) the docs pages — assembled from the same hopSection lines, no second render site. No must-fix.
  • Should-fix (follow-up sweep, not a FAIL): the PR drops the note from the one entry the card names and leaves the same note, word for word, in 219 sibling semantic entry files on main, and leaves compliance-families-retirement.test.ts:260-262 asserting not.toMatch(/BACKTICK/) on entry.surface for the same stale reason while it drops the identical assertion from its two siblings. None of this breaks a byte of what ships, and the kept assertion still passes (those surfaces carry no backtick), but every one of those comments now states something false about the renderer and the kept test refuses a future backtick for a reason that no longer exists. One sweep card: drop the 219 comments and the one assertion together; the card body should correct "Only 18.visibility-predicate-… avoids this" to the measured 219.
  • Should-fix (prose, outside the hits, for the seat before landing): strike the reviewer-model name from the PR body's independent-review line (③).

Implemented-by: claude/project-thread-2ul8sb
Reviewed-by: session_01L6bhgV4fMGJHSdSdx3u9mK

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:spec#4 · marchtian · session_017ihL2fi9zfJhUTubyJYb6V

Late record, posted by the seat after landing: the card thread (session_017ihL2gP8YEs7CoWrbVsSAi) that was stood down when the Fable quota ran out had already run these three reviews on the content that merged here. The seat's takeover added only the origin/main merge, and the seven files are byte-identical to 85151844. The 178-file workaround-comment sweep is #27731. Text below is the card thread's draft, unchanged.

Independent full Fable review of this PR (AGENTS.md: every spec, skills or ADR change carries one before merge). Run as an isolated subagent (model fable, effort xhigh) with the card, the order and the whole diff; three rounds, each verdict quoted verbatim below. Posted 2026-10-31T12:09Z.

Round 1 — head 94a629e (fix commit 8e4d77a merged over origin/main 0e33108)

Verdict: PASS-WITH-SHOULD-FIX

The card's defect is fixed where the ruling says (the renderer, once), every one of the 624 real surfaces now reads back verbatim in two independent CommonMark/GFM parsers, the pin reads the real generator's bytes and its lit control bites, and no entry is hand-edited. Two things are left undone around the edges: the PR's own extra claim (the | escape in table rows) is not actually pinned, and the same workaround note the card asked to drop survives as an assertion in two sibling test files the seat did not find.

Must-fix: none.

Should-fix 1 — the conversion-row pin cannot see the pipe defect it claims to cover; it passes on the pre-fix bytes (the reviewer ran the test's row logic over the guide generated by the OLD generator: 103 pass / 0 fail, while micromark + gfm-table splits the two pipe-carrying rows into 6 cells).
Should-fix 2 — the workaround the card asks to drop survives as an assertion in src/api/export-job-family-retirement.test.ts and src/kernel/package-upgrade-family-retirement.test.ts (comment plus expect(entry.surface).not.toMatch(backtick)), both present at the merge base; after this PR the stated reason is false and the pin forbids what is now legal.

Nits (9): "byte-identical for a backtick-free text" overstated (a leading or trailing space pads); the line-ending reason in the codeSpan docblock half wrong (a list item lazily continues, a table row ends); surfaceSpan a passthrough with a third copy of the explanation; the loud refusal does not name the entry; a dead ?? [] branch in the test; two reader deviations from CommonMark unreachable from the encoder (a lone CR, a tab-only span); tableRow's escape not injective for a cell already holding backslash-pipe (zero today); no test-budget line owed (5.01 s wall); lint not in the seat's pipeline (the reviewer's own eslint run on the touched files: exit 0).

Verified: the ruling is satisfied (one helper, both sites, the only src/ change is the two-line comment deletion, no surface text touched); rendering correctness against two real parsers (markdown-it 14.3.3 and micromark 4.0.2 + gfm-table, the remark-gfm stack fumadocs runs): new rendering 0 mismatches of 624, old rendering 183 breaks in both parsers plus 2 split rows; the old and new single-file guides differ in exactly 185 of 1887 lines (183 semantic lines, 2 table rows); the pin spawns the same command the publish lane runs and reads its bytes; 175 of 333 unmarked surfaces carry Markdown/MDX-active text, so the longer fence over prose is the defensible choice; changeset level and both declaration lines read declared through scripts/pm/clause2-line.mjs; no generated artifact under packages/spec must move; license headers, naming, trailer pair all in the required form.

Delta — d5876c7 on 94a629e (every round-1 finding applied)

Verdict: PASS-WITH-SHOULD-FIX

Every round-1 finding is applied, and applied correctly: the conversion-row pin now fails on the pre-fix bytes (2 of 103) and passes on HEAD (103 of 103) with cell counts that agree with micromark on every row; the two sibling workaround pins are gone; the guide's bytes are unchanged by this commit. Two new things, one of them a round-1 miss of my own: the same workaround note the card asked to drop lives in 178 semantic entry files (which the binding ruling says this PR must not hand-edit, so it needs disclosure and a follow-up card, not a diff), and the new lit control's piped.length > 0 floor rests on two retirement kits that ADR-0087 schedules for removal at protocol 20.

Must-fix: none.

Should-fix 1 — the workaround note survives in 178 semantic entry files (12 under major 17, 166 under 18; 179 at the merge base) and the PR does not say so; the binding ruling keeps the sweep out of this PR, so the remedy is an acceptance note plus a follow-up card (or the maintainer's word that the sweep rides here), and the card's own "only 18.visibility-predicate-… avoids this" is wrong by 178.
Should-fix 2 — the new lit control's liveness floor (piped.length > 0) is scheduled to go red: the only two pipe-carrying conversions are retirement kits that leave the registry when protocol 20 is cut; make the control registry-independent (a literal old-style row with a pipe) and drop the floor on the real-registry loop.

Nits (3): the round-1 count floors (> 100, > 400, > 50) are magic numbers a later tree can cross for no defect, > 0 says all the liveness needed; not.toContain('\\') is broader than the ambiguity, only backslash-pipe is where GFM renderers part ways; "both registries are gitignored" in the brief is half right (conversions' is tracked).

Verified: the guide's bytes at d5876c7 are identical to round 1's; the row pin simulated with the test's gfmCells copied verbatim gives pass 101 / fail 2 on the pre-fix guide and 103 / 0 on HEAD, 0 cell-count disagreements with micromark on either; the four new reader edge cases match markdown-it exactly; workaround-note census by fixed-string grep; kit lifetime from ADR-0087's 2026-10-26 addendum and the two conversions' retiredAfter fields; callers of codeSpan/surfaceSpan are only the generator (both passing the id) and the test; changeset lines still declared; eslint on the five touched files exit 0.

Delta 2 — 8515184 on d5876c7 (the delta's findings applied; test file only)

Verdict: PASS with nits

One file, packages/spec/scripts/upgrade-guide-surface-span.test.ts (16+/13−). The commit does what it says: the pipe lit control starts from a literal old-style row and then loops the real registry with no floor; four count floors drop to > 0; the conversion-surface pin forbids the pair backslash-pipe instead of any backslash. Nothing in it is wrong; two observations below are nits.

(a) The literal control exercises the same gfmCells and the old generator's exact row shape (four cells, id and surface each in a single-backtick span, one unescaped pipe inside the surface span); gfmCells and micromark + gfm-table both read it as 5 cells, so the control still bites after the two pipe-carrying retirement kits leave the registry at protocol 20.
(b) > 0 on the real-set pins is the liveness AGENTS.md asks of a self-test; the one vacuous shape they do not cover (every registry major at or below the support floor) is owned by src/migrations/migrations.test.ts, and rendered() is loud on a major above PROTOCOL_MAJOR with no section. Nothing to add.
(c) The regex matches exactly a backslash immediately before a pipe (measured matrix), and the comment's two readings are real: micromark + gfm-table reads the re-escaped cell as a literal backslash plus a cell boundary, markdown-it as a backslash plus an escaped pipe; a backslash anywhere else reads the same in both. Caveat: cmark-gfm, GitHub's own renderer of the Release attachment, was not run in any round.
(d) The commit message does not overstate; "age out one major on" is the ADR's own phrase.

Nits (2): the pin also refuses a double backslash before a pipe, which both parsers read the same way, "not worth a line"; "GFM renderers" in the comment rests on the two renderers measured (micromark, markdown-it), cmark-gfm unmeasured.

Verified: the full commit and its parent; gfmCells copied byte for byte into an independent run; the literal row and five backslash/pipe surfaces through tableRow's escape and both parsers; the regex matrix; the floor pins in migrations.test.ts; the ADR-0087 kit-lifetime wording; eslint on the test file exit 0; the seat's run read (15 of 15 green).

Disposition

Every must-fix: none in any round. Every should-fix and nit from rounds 1 and 2 is applied in the two follow-up commits, except the 178-file comment sweep, which the binding ruling (triage 6496306402, "the 516 entries are not hand-edited") keeps out of this PR: it is disclosed in the PR body's acceptance notes and handed to the seat as a follow-up decision in the os-dev report on #27532. The two delta-2 nits are wording in one test comment; the reviewer called them not worth a line, so no fourth commit: the PR body's acceptance note names the two renderers measured and that cmark-gfm was not.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

1 participant