fix(readiness): supported API paths to clear ADR-079 readiness findings

[thewrz]

Why

PR 544 (ADR-079) added the issuance-readiness gate: a final-mode issuance blocks
with a 422 on any outstanding unresolved_choice_token, specifier_note_present,
open_comment, or body_object_present finding. ADR-079's "block, never strip"
decision says a blocked issuance returns its findings so a human can fix the
source and retry — but for three of the four finding kinds there was no
supported API path that actually cleared the finding on an existing spec.
"Fix the source and retry" was aspirational, not actual. The only working
escape hatch was overrideReadinessGate, an explicitly unaudited bypass.

Closes #545.

What

All three supported clearing paths, per the issue's settled scope:

• Path 1 — re-derive choiceTokens on text edit. applyParagraphUpdate
  now re-derives only the choiceTokens key of source_facts from the new
  text (reusing the parser's existing scanChoiceTokens, exported through the
  parser barrel — no second detector), in the same statement/transaction as the
  text write. Every other source_facts key is spread through untouched;
  comment closure is explicitly NOT re-derived from text (it's an OOXML
  artifact that doesn't survive round-trip).
• Path 2 — a per-node acknowledgement. New paragraphs.acknowledged
  boolean (migration 055) clears specifier_note_present /
  body_object_present WITHOUT hiding or removing the note/text-box content —
  a separate, additive piece of state, never the vanish mechanism (vanish is
  deliberately excluded for note/object — see the comment above
  REMOVABLE_NODE_TYPES). New endpoint: PATCH /specs/:id/paragraphs/:nodeId/acknowledgement.
• Path 3 — a mutable comment-closure toggle. New endpoint: PATCH /specs/:id/paragraphs/:nodeId/comments/:index/closure. acceptCommentAsNote
  now also closes the originating comment in the same transaction as the note
  insert — previously it left the comment open, so accepting it strictly
  increased the number of blocking findings (a new note plus a still-open
  comment). That's the sharpest symptom in the issue, and it's fixed.

openapi.yaml and the MCP tool surface (acknowledge_paragraph,
set_comment_closed, both write tier) are updated in lockstep; the INV-6
write-pending ratchet baseline moves 62 → 64, a documented genuine scope
increase.

Design decisions (no ADR — sprint policy)

• Acknowledgement is a separate boolean, never merged into
  REMOVABLE_NODE_TYPES or the vanish mechanism — the existing comment above
  that set explains precisely why storing vanish on note/object would
  silently lie about the removal contract (both renderers emit/consider those
  types before ever checking vanish). No renderer may ever consult
  meta.acknowledged — enforced by a new byte-for-byte render-parity test
  (see mutation-verification below).
• runAccept's comment-closure sub-write on the anchor gets its own
  close-comment history row and its own base_version bump, sharing the
  outer write's already-resolved content_version (one generation, two
  paragraph_versions rows: the note insert + the anchor's closure) — per
  paragraph-history.ts's own contract that every content-mutating write
  records history for every paragraph it touches. The pre-existing gate-free
  idempotent-repeat fast path (findExistingNoteByProvenance) is left
  untouched; it writes nothing today and this fix doesn't change that.
• closeCommentHandler's no-comment result maps to 404 (a lookup miss),
  not acceptAsNoteHandler's 422 (a validation failure) — there's no content
  to validate here, only a location to find.
• New paragraph-comment-closure.ts is deliberately not named
  comment-closure.ts, to avoid a same-basename collision with the unrelated
  read-only src/parser/docx/comment-closure.ts / src/ast/comment-closure.ts.
• buildSubtree/buildSubtreeMeta/SubtreeRow were split out of
  paragraphs.ts into a new paragraph-subtree.ts companion file — threading
  acknowledged through pushed that file over the repo's enforced
  max-lines: 400 cap. router.ts got the same treatment via a new
  registerParagraphClearanceRoutes extraction.

PR size

This PR is ~1870 LOC across 7 commits, over the repo's 500-LOC advisory
loc-check — expected and pre-approved for this issue: three independent,
fully-tested clearing paths spanning DB/API/MCP/OpenAPI plus their own
integration coverage, per the sprint's "the PR grows; that is fine" policy.

Mutation-verification transcripts

Render-parity assertion (src/generator/acknowledgement-render-parity.test.ts)
— the assertion this path most risks per the issue's own verification bar.
Temporarily branched src/generator/markdown.ts's note-rendering on
node.meta.acknowledged during the design spike (reverted before real
implementation):

- markdown output is byte-identical acknowledged vs unacknowledged
  AssertionError: expected '...different string when acknowledged...'
  to equal '...unacknowledged markdown...'
  ✗ FAILED — confirms the assertion is not vacuous

Reverted the mutation; suite returned to green. The shipped test asserts
parity across markdown, .SEC, AND DOCX (unzipped word/document.xml
string comparison — Packer.toBuffer() embeds a fresh zip/docProps
timestamp on every call, so whole-Buffer equality is unusable even for two
generations of the identical tree; confirmed experimentally).

unrelated source_facts keys survive a text edit assertion
(src/db/queries/source-facts-rederive.test.ts) — temporarily had
deriveNextSourceFacts drop unrelated keys (return only the recomputed
choiceTokens) instead of spreading existing through:

- unrelated source_facts keys survive a text edit byte-identical
  AssertionError: expected undefined to equal [{ author: 'Jane', ... }]
  ✗ FAILED — confirms the assertion is not vacuous

Reverted the mutation; suite returned to green.

Adversarial cross-review (Codex gpt-5.6-sol, xhigh)

Ran once as the final draft gate. 2 findings, both [P1], both handled —
full assessment, reproduction transcripts, and the mutation-verification
matrix are in a
dedicated PR comment.
Summary:

• acknowledged was silently stripped by SpecTreeSchema (confirmed).
  SpecNodeMetaSchema never mirrored the new SpecNodeMeta field and has no
  .catchall(), so validateTree — which sits on the package-issuance and
  revision-freeze paths — dropped it: an acknowledged note re-blocked a
  package issuance and frozen snapshots lost the state. The single-spec
  /generate path skips validateTree, which is why the end-to-end test
  still passed and the bug hid. Same bug class as generator: round-trip drops manual page breaks (w:br type="page"), collapsing multi-page fixtures to one page #497's pageBreakBefore.
  Fixed in 7d2b2d4e.
• An acknowledged note bypassed its own other blockers (refuted as
  stated — a note branch never emitted those findings on main either — but
  the bypass it points at is real and is introduced here: pre-ADR-079 readiness gate: no supported edit-API path to clear choice-token/note/open-comment findings #545 the note
  always blocked, so an open comment on a note now reaches a final issuance).
  Acknowledgement now clears only specifier_note_present. Fixed in
  7d2b2d4e.

Both fixes are pinned by regression tests named for the symptom, and both new
gates were mutation-verified alongside the four pre-existing ones.

Testing

• Unit tests pass — pnpm test: 258 files, 3650 tests, 0 failures.
• Integration tests pass — pnpm test:integration: 159 files, 1822 tests
  passed, 141 skipped (pre-existing fixture-conditional skips), 0 failures.
• Migration 055 applies and rolls back cleanly (pnpm migrate /
  pnpm migrate:down, verified against a real Postgres instance) —
  corrected during review: the original down re-narrowed
  paragraph_versions_op_check without first removing the rows the
  widened list had allowed, so it hard-failed
  (check constraint ... is violated by some row) the moment the feature
  had actually been used. Fixed in 98bcf979 (mirrors migration 029's
  down) and re-verified red→green with seeded acknowledge /
  close-comment history rows present.
• Manual verification: the full end-to-end scenario required by the issue
  — a final-mode issuance blocked by all four finding kinds, cleared
  through the three supported API paths ALONE with overrideReadinessGate
  unset, now succeeds — is asserted in
  src/api/readiness-clearance.integration.test.ts ("end-to-end: a
  final-mode issuance blocked by all four finding kinds succeeds..."),
  against a real generated DOCX (zip) payload.
• pnpm lint (eslint + tsc --noEmit + prettier --check) green.
• CI green (all six checks passed on the pre-review head; re-running on
  the two review-fix commits).

🤖 Co-authored by Claude Sonnet 5. Closes #545.

Summary by CodeRabbit

• New Features
  • Added per-paragraph acknowledgement for notes and text boxes, allowing accepted content to be marked without removing it from rendered documents.
  • Added controls to close or reopen individual source-document comments.
  • Added acknowledgement and comment-closure tools for supported integrations.
• Bug Fixes
  • Final-mode generation now accurately reflects resolved findings and continues reporting unrelated issues.
  • Accepting a comment as a note now closes the original comment automatically.
• Tests
  • Expanded coverage for readiness checks, rendering consistency, validation, and integration workflows.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d7470ec3-cbf8-4fb5-8eac-3c3f8ecfc958

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Added per-paragraph acknowledgement and comment-closure operations. Persisted acknowledgement state now affects readiness findings without changing rendered output. REST and MCP interfaces expose idempotent, edit-gated mutations with ownership and node validation. Paragraph edits rederive choice-token facts.

Changes

Readiness clearance

Layer / File(s)	Summary
Contracts and persisted acknowledgement state openapi.yaml, src/ast/*, src/db/migrations/*, src/db/queries/paragraphs*, src/db/queries/specs.ts, src/db/queries/revision-snapshot.ts, src/db/queries/paragraph-subtree.ts	The API and schema contracts define acknowledgement and comment-closure payloads. Paragraph persistence, history constraints, queries, and SpecNode.meta now carry acknowledgement state.
Source-fact rederivation src/db/queries/source-facts-rederive.ts, src/db/queries/paragraphs.ts, src/parser/*	Paragraph text updates preserve unrelated source facts and rederive choiceTokens from the new text.
Database mutations src/db/queries/paragraph-acknowledgement.ts, src/db/queries/paragraph-comment-closure.ts, src/db/queries/reclassify.ts	Added transactional, idempotent acknowledgement and comment-closure updates with ownership checks, edit gates, history entries, and content-version updates. Accepting a comment as a note now closes the original comment in the same transaction.
REST and MCP interfaces src/api/*, src/mcp/*	Registered acknowledgement and comment-closure routes and tools. Handlers validate inputs and map database, ownership, missing-resource, node-type, edit-gate, and server-error outcomes.
Readiness and integration validation src/lib/readiness-review.ts, src/api/readiness-clearance.integration.test.ts, src/mcp/paragraph-tools.integration.test.ts, src/generator/acknowledgement-render-parity.test.ts, src/*/*.test.ts	Tests cover readiness clearance and restoration, source-fact updates, schema round trips, history, REST and MCP flows, and unchanged Markdown, .SEC, and DOCX output.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant REST API
  participant Database
  participant Readiness Review
  participant Generator
  Client->>REST API: Acknowledge node or close comment
  REST API->>Database: Apply validated mutation
  Database-->>REST API: Return updated SpecNode
  REST API-->>Client: Return mutation response
  Client->>Readiness Review: Request final readiness
  Readiness Review-->>Client: Return cleared findings
  Client->>Generator: Request final generation
  Generator-->>Client: Return DOCX payload

Loading

Possibly related issues

• None. The retrieved issue does not describe the acknowledgement and comment-closure implementation in this PR.

Possibly related PRs

• wrzonance/SpecR#185: Adds the scanChoiceTokens and SourceFacts.choiceTokens flow reused for rederiving choice-token findings.
• wrzonance/SpecR#272: Defines the shared comment-closure state and SourceFacts handling extended by these write operations.
• wrzonance/SpecR#197: Provides the edit-gate, row-locking, and content-version infrastructure used by the new paragraph mutations.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 53.06% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the PR's main change: adding supported API paths to clear ADR-079 readiness findings.
Linked Issues check	✅ Passed	The PR satisfies issue #545 by adding supported paths for choice tokens, notes, body objects, and comments, with persistence and regression coverage.
Out of Scope Changes check	✅ Passed	The changes support the linked issue through REST, MCP, database, schema, readiness, migration, and test updates without unrelated scope.

---

_{Comment @coderabbitai help to get the list of available commands.}

[thewrz]

This was written agentically; verify its assertions and edit accordingly:

Adversarial cross-review — Codex gpt-5.6-sol (effort xhigh)

Ran once as the final draft gate against origin/main, with --ignore-user-config. 2 findings, both [P1], both handled. Each was verified against running code before any fix; neither was auto-applied.

---

[P1] acknowledged is stripped at the SpecTree validation boundary — CONFIRMED, fixed in 7d2b2d4e

Adding this field only to the TypeScript interface causes SpecTreeSchema to silently strip it because SpecNodeMetaSchema has no corresponding key.

Correct, and the more serious of the two. Reproduced directly rather than taken on faith:

meta after SpecTreeSchema.parse: {}
findings BEFORE validateTree: []   AFTER: [ 'specifier_note_present' ]

acknowledged was added to SpecNodeMeta (ast/types.ts) but not mirrored in SpecNodeMetaSchema (ast/spec-tree-schemas.ts), which has no .catchall() — so z.object dropped it on every validation path. validateTree (db/queries/revision-snapshot.ts) sits on the package-issuance and revision-freeze paths (revisions.ts, reporting.ts), so an acknowledged note re-blocked a package issuance and frozen snapshots lost the state entirely. The single-spec POST /specs/:id/generate path does not go through validateTree, which is why the end-to-end test still passed and the bug hid.

This is the exact failure mode that schema's own comment warns about ("a field added to the type but not mirrored here would silently vanish on every JSONB round-trip"), and the same bug class #497 fixed for pageBreakBefore. Fixed by mirroring the key; pinned by preserves acknowledged through validation (#545) and a stronger readiness: acknowledgement survives a SpecTreeSchema round-trip (#545) that asserts the cleared finding state survives, not merely the key.

[P1] Acknowledged note bypasses other blockers — refuted as written, but a real bypass it exposed is fixed in 7d2b2d4e

When a note also carries sourceFacts.comments or choiceTokens, this early return makes acknowledgement clear open_comment and unresolved_choice_token findings too.

The stated mechanism is wrong. On main, assessNode's note branch returned [specifier_note_present] and never fell through to choiceTokenFindings/openCommentFindings — so those findings were never emitted for a note node, before or after this PR. Acknowledgement does not "clear" findings the gate previously reported.

The underlying concern is nonetheless real, and this PR does introduce it: pre-#545 a note always blocked, so its own unresolved facts never needed a guard. Acknowledging it now removes the only blocker, letting an open comment or unresolved choice token carried on that note reach a final-mode issuance. Acknowledgement is documented to clear only specifier_note_present, so the bare [] was too broad. An acknowledged note now still reports its own open_comment / unresolved_choice_token — both of which have their own supported clearing paths (comment closure / text edit), so this is hardening, not a new dead end.

---

Also found and fixed independently of Codex — migration 055 was not reversible (98bcf979)

down re-narrowed paragraph_versions_op_check to the pre-055 op list without first removing the rows the widened list had allowed. Once the feature was actually used, pnpm migrate:down hard-failed against a live database:

ERROR: check constraint "paragraph_versions_op_check" is violated by some row  (ATRewriteTable)

The PR body's "rolls back cleanly" claim only held on an unused schema. Fixed by deleting the four added ops' history rows before re-narrowing, mirroring migration 029's down. Verified red→green: fails with seeded acknowledge/close-comment rows before the fix, down → up round-trips after.

Mutation verification

Every gate on this PR was verified to actually fail when its invariant is broken — the two the issue flagged as easiest to write vacuously, plus the two added here:

Gate	Mutation	Result
render parity — markdown	branch note rendering on meta.acknowledged	✗ fails
render parity — DOCX	branch emitNode on meta.acknowledged	✗ fails
render parity — .SEC	branch renderNote on meta.acknowledged	✗ fails
unrelated source_facts survive	deriveNextSourceFacts drops the spread	✗ fails
acknowledged survives validation	remove the schema key	✗ fails
ack is not a blanket bypass	restore the bare return []	✗ fails

Also re-confirmed: note/object are still absent from REMOVABLE_NODE_TYPES, vanish is not reused, and readiness-review.ts remains the only consumer of meta.acknowledged — no renderer or merge code reads it.

🤖 Co-authored by Claude Opus 5.

[thewrz]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@openapi.yaml`:
- Around line 709-770: Update the ParagraphHistoryEntry.op enum in openapi.yaml
to include acknowledge, unacknowledge, close-comment, and reopen-comment,
ensuring paragraph-history responses accept all history operations created by
these endpoints.

In `@src/mcp/paragraph-tools.integration.test.ts`:
- Around line 570-578: Clean up the paragraph acknowledgement integration test
by removing the repeated onNode declarations in the same scope so only one
declaration remains. In the type literals used around
handleAcknowledgeParagraph, remove duplicate meta members and retain a single
meta definition per type while preserving the existing assertions.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f68ea949-d433-46db-bb39-4345f4c7a53a

📥 Commits

Reviewing files that changed from the base of the PR and between 14f06e3 and 7d2b2d4.

📒 Files selected for processing (39)

• openapi.yaml
• src/api/contract.integration.test.ts
• src/api/editability.ts
• src/api/paragraph-acknowledgement.ts
• src/api/paragraph-clearance-routes.ts
• src/api/readiness-clearance.integration.test.ts
• src/api/router.ts
• src/ast/index.ts
• src/ast/schemas.test.ts
• src/ast/spec-tree-schemas.ts
• src/ast/types.ts
• src/db/index.ts
• src/db/migrations/055_paragraph_acknowledgement.ts
• src/db/queries/paragraph-acknowledgement.ts
• src/db/queries/paragraph-comment-closure.test.ts
• src/db/queries/paragraph-comment-closure.ts
• src/db/queries/paragraph-history.test.ts
• src/db/queries/paragraph-history.ts
• src/db/queries/paragraph-subtree.ts
• src/db/queries/paragraphs-batch.ts
• src/db/queries/paragraphs.test.ts
• src/db/queries/paragraphs.ts
• src/db/queries/reclassify.ts
• src/db/queries/revision-snapshot.ts
• src/db/queries/source-facts-rederive.test.ts
• src/db/queries/source-facts-rederive.ts
• src/db/queries/specs.test.ts
• src/db/queries/specs.ts
• src/generator/acknowledgement-render-parity.test.ts
• src/lib/readiness-review.test.ts
• src/lib/readiness-review.ts
• src/mcp/capabilities.ts
• src/mcp/contract-map.ts
• src/mcp/contract-write-response-map.ts
• src/mcp/paragraph-handlers.ts
• src/mcp/paragraph-tools.integration.test.ts
• src/mcp/paragraph-tools.ts
• src/parser/docx/index.ts
• src/parser/index.ts