Assessment domain: GA evidence appendix renderer (PRD-0016) + implementation sketch#129
Conversation
Thin PRD for the design sketch's first build step: a pure renderer from PageVerificationReport (+ optional StabilitySignal) to the pasteable Talk:Article/GAn wikitext evidence appendix. Wording invariants as contract (no pass/fail, PRD-0014 mismatch framing), plain wikitext with nowiki- escaped quotes, stable ordering for hold-loop comparability, provenance footer, assessment-domain placement per the layering rule. Three open questions with proposed answers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…aced Four-phase plan grounded in the tree: sp42-assessment crate with a pure wikitext builder over PageVerificationReport (bypassing ReportDocument, whose flat-string shape carries none of the appendix's invariants), CLI ga-appendix format plus a pure render-report subcommand, fixture realism, and a staged stability phase landing with PRD-0015's implementation. Feeds three learnings back into PRD-0016: the grounding axis joins the wording invariants (Unlocated support renders as unconfirmed), the stability DoD items are staged so the citations-only appendix is the MVP gate, and the saved-report mode is the core surface (no bridge, session, or network) with the verify-page flag as convenience. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
Both surfaces ship (saved-report render as the core, verify-page flag as convenience); supported findings render as a compact one-line-each spot-check record rather than counts-only, with unconfirmed supports in their own sublist; the not-assessed line stays, with criterion 5 leaving it exactly when a StabilitySignal renders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…tion Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
The default reader has zero SP42 context — a pasted appendix is read cold by reviewers, nominators, and watchers, and is most readers' first SP42 artifact. Consequences: cold-reader legibility joins the wording invariants (no raw contract identifiers, copy-module vocabulary, human-facing citation markers), the provenance footer gains a what-is-this explainer link with a DoD assertion, and the copy-drift risk is reweighted as first impression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…-first sublists Criterion numbering is now defined at first use with the GA-criteria link; appendix section headings carry number and name for the cold reader; the criterion-2 bullet becomes a numbered section spec ordered by consequence for the review (claim-source disagreements lead, link rot follows, honesty arms and books close), with refs addressed by their human-facing citation markers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…target; criterion-5 heading The complement enumeration (1a, 2c, 3, 4, 6) was wrong — it omitted 1b, 2a, and 2d — and re-drifts as evidence lanes land, so the line now states the tiny assessed set positively (2b only; plus 5 when a StabilitySignal renders). Explainer link gets an MVP target (repo docs via the platform public_documents surface; on-wiki essay as the eventual home). Criterion 5 heading carries its name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…noted The stats arm lacks a grounded/unconfirmed split within supported verdicts, so the summary line derives it deterministically from findings rather than blending them; an additive supported_unlocated counter on PageVerificationStats is noted as the clean upstream fix for the references domain, not gated on here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…d, placement corrected Plain wikitext is the dependency-failure-mode choice, not renderer trust; the native GAList idiom counterargument is recorded and routed to the alpha copy review. Sublist categories order by consequence while findings keep report order within each. Placement corrected: PageVerificationReport lives in sp42-citation (domain-to-domain dependency), StabilitySignal slated for sp42-core, crate flat per the actual workspace layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…r wording The report contract carries no run timestamp, so rendered_at is a shell-injected Clock input pinned in the replay test, the footer labels its date as the render date, and an additive verified_at on the report is the second noted upstream fix. Resolved Q1 gains its DoD binding (both CLI surfaces tested, saved-report render byte-identical). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
…ationale Phase 1's section spec now matches the reviewed PRD (consequence order, supported compact list, assessed-by line, headings, legibility scan, injected rendered_at); phase 3 carries the GAList question to reviewers; a review-time sync section records the flow-back and the two upstream notes. The sp42-reporting alternative's rationale corrected: it hosts shared report primitives, not patrolling-only reporting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a2d12cb80
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| are addressed by their human-facing citation markers (`[1]`, named refs), | ||
| which is what `ref_id` already carries (ADR-0011). |
There was a problem hiding this comment.
For the saved-report renderer, this assumption is not true: the extractor has BlockRef::ref_text for the visible marker, but extract_use_sites only copies ref_id into CitationFinding/SkippedRef, and ref_id is the stable cite id (e.g. cite_ref-smith_3-0), not [1] or the rendered named-ref marker. Since this PRD also commits to rendering from a stored PageVerificationReport with no page fetch, the renderer cannot recover the human-facing marker unless the report contract carries ref_text (or the output requirement is changed), and following the current text would violate the cold-reader/no-raw-identifier invariant.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against the code and fixed in b9e537d — ref_id is indeed the stable cite id and BlockRef::ref_text never reaches the report. The PRD now specifies a derived reader-facing ref label for the MVP (ref name parsed from the cite id when present, else a stable per-report index; raw cite_ref-… ids join the no-raw-identifiers DoD scan), with an additive ref_text on CitationFinding/SkippedRef recorded as an upstream note for the references domain.
Generated by Claude Code
| 2. *Dead links* — unreachable sources, with their `archive_of` repair | ||
| handles. Mechanical and actionable. |
There was a problem hiding this comment.
In the current report contract, archive_of is only populated when a dead live URL was successfully verified through an archive fallback; unresolved SourceUnavailable(Unreachable) findings generally have archive_of = None, and the citation's fallback archive URLs are not preserved in CitationFinding or PageVerificationReport. With this spec, a saved-report renderer cannot make every dead-link line carry archive_of repair handles, and it risks mixing recovered-via-archive citations with unrecovered link rot; either require a report field for fallback candidates or scope archive_of rendering to findings that actually came from an archive fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in b9e537d — archive_of only exists on findings actually recovered through an archive fallback, and unrecovered Unreachable findings carry no archive candidates (those stay on the extractor's use-site). The spec now splits the sublists: recovered-via-archive (supported, with the archive_of repair handle, pulled out of the supported list because they're actionable) vs. unrecovered dead links (dead URL only), and an additive candidate-archives field on unreachable findings is recorded as an upstream note.
Generated by Claude Code
ref_id is the stable cite id, not the rendered marker: the MVP derives a reader-facing ref label (raw cite_ref ids join the no-raw-identifiers scan) and an additive ref_text on CitationFinding/SkippedRef is noted upstream. archive_of exists only on findings recovered through an archive fallback: the dead-links sublist splits into recovered-via-archive (repair handles) and unrecovered (dead URL only), with an additive candidate- archives field noted upstream. Sketch synced; upstream notes now number four. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9e537d0a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| wording invariants (incl. `Unlocated` rendering as unconfirmed support and | ||
| a no-raw-contract-identifiers scan), nowiki escaping over a | ||
| malicious-quote fixture, determinism with a pinned timestamp. | ||
| 2. **CLI surface.** `OutputFormat::GaAppendix` accepted by `verify-page`; new |
There was a problem hiding this comment.
When this phase adds OutputFormat::GaAppendix, the current sp42-cli/src/main.rs enum is annotated #[value(rename_all = "lowercase")], so a bare variant addition will expose gaappendix, while the PRD/DoD asks reviewers to type --format ga-appendix. Please call out an explicit value name or a rename_all change here; otherwise the advertised CLI and the planned CLI tests will not match the parser.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified (main.rs:37 carries #[value(rename_all = "lowercase")]) and fixed in 0973da5: the sketch's CLI phase now pins an explicit #[value(name = "ga-appendix")] on the new variant, leaving the existing single-word values untouched.
Generated by Claude Code
| 5. *Unconfirmed supports* — supported/partial verdicts whose quote could not | ||
| be re-located: the panel's judgment without evidence in hand, never | ||
| blended into the supported list. |
There was a problem hiding this comment.
A Partial finding with grounding_status = Unlocated currently fits two mutually exclusive buckets: the earlier disagreement bucket includes every Partial, while this bucket also includes supported/partial verdicts whose quote could not be re-located. The verifier can produce that exact combination when a winning partial/support quote does not locate, so an implementation can either duplicate the finding or hide a claim-source disagreement under “unconfirmed supports”; please make the precedence explicit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real ambiguity — fixed in 0973da5 with explicit precedence: verdict partitions the sublists, grounding annotates. Every NotSupported/Partial finding renders in the disagreements bucket regardless of grounding status (a Partial+unlocated carries an unlocated annotation on its line there — hiding a claim–source disagreement is the worse failure), the unconfirmed-supports sublist narrows to Supported+unlocated only, and the DoD now asserts both combinations plus every-finding-in-exactly-one-sublist.
Generated by Claude Code
| answer was revised during discussion: the GA spot-check's purpose is to | ||
| document that sources *support* the text, and a reviewer's record must say | ||
| *which* refs were checked — counts alone don't serve that. Supported | ||
| findings render one line each (`ref_id`, claim prefix, grounding marker), |
There was a problem hiding this comment.
Fresh evidence after the earlier ref-label review is this remaining Q2 summary: it still says supported entries render ref_id, even though the main body now says CitationFinding.ref_id is only the stable cite id and raw cite_ref-… ids must not appear. If implementers follow this resolved-question text, supported findings will violate the cold-reader/no-raw-identifier invariant and the DoD scan, so this should say the derived reader-facing ref label instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — a residual from the earlier ref-label fix. Q2's resolution text now says "derived reader-facing ref label" (0973da5).
Generated by Claude Code
…, CLI value name Verdict partitions the criterion-2 sublists and grounding annotates: a Partial+unlocated finding stays in the disagreements bucket with an annotation (hiding a disagreement is the worse failure), Supported+unlocated goes to unconfirmed supports, and every finding appears in exactly one sublist, with the DoD testing both combinations. Q2's resolution text drops its residual raw ref_id. The sketch pins an explicit ga-appendix value name against OutputFormat's lowercase rename_all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0973da56e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 1. *Claim–source disagreements* — `NotSupported`/`Partial` findings, each | ||
| line carrying the claim, its reader-facing ref label, the reader-facing | ||
| verdict, the verbatim located quote, and the source link. These can sink |
There was a problem hiding this comment.
The saved-report renderer cannot satisfy this for all disagreement findings: GroundingStatus::NotApplicable is explicitly used when no supporting quote is expected for NotSupported, and assemble_citation_finding returns a no_quote_finding(..., GroundingStatus::NotApplicable, ...) for non-support winners, so CitationFinding.passage is absent and the report does not retain panel votes. For NotSupported and unlocated Partial inputs, requiring a verbatim located quote will either make the renderer tests impossible to satisfy or push implementers to present source_excerpt as if it were grounded evidence; scope the quote requirement to findings that actually carry passage and define the fallback wording for the rest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed (no_quote_finding with NotApplicable at verify.rs:562 — ADR-0007's no-quote verdict semantics) and fixed in 92ca05b: the quote requirement is scoped to findings carrying passage; a no-quote NotSupported line states that no supporting passage was found; and source_excerpt may only ever render as labeled context (what the panel read), never presented as grounded evidence — with a DoD item asserting exactly that.
Generated by Claude Code
| 2. *Recovered via archive* — findings whose claim was verified through an | ||
| archive fallback: supported, but the live URL is dead, and `archive_of` | ||
| (which the contract populates **only** in this case) is the repair | ||
| handle — "update the citation to the archive." Pulled out of the | ||
| supported list because they are actionable. |
There was a problem hiding this comment.
Because the PRD says the disagreements bucket takes every NotSupported/Partial finding, archive-backed disagreements will never reach the recovered-via-archive bucket, but try_archive_fallback stamps archive_of on any archive result whose verdict is not SourceUnavailable, including Partial and NotSupported. In that scenario the appendix would show the archive source link while losing the dead live URL repair handle, so require archive_of to render on any line that carries it, not only on supported recovered-via-archive entries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed (try_archive_fallback accepts any non-SourceUnavailable archive verdict) and fixed in 92ca05b: archive_of now renders on any line that carries it, in every bucket — archive-backed disagreements keep their dead-live-URL repair handle in the disagreements bucket, recovered-via-archive narrows to Supported, and a DoD item covers the archive-backed-disagreement case.
Generated by Claude Code
| 5. *Unconfirmed supports* — `Supported` verdicts whose quote could not be | ||
| re-located: the panel's judgment without evidence in hand, never blended | ||
| into the supported list. (`Partial`+unlocated stays in the disagreements | ||
| bucket, annotated — see its precedence rule.) |
There was a problem hiding this comment.
This only defines the unconfirmed bucket for Supported+Unlocated, but LocatedFuzzy is also a real grounding status and is explicitly not groundable (is_groundable_support only accepts exact Located); the existing presentation helper groups Supported | Partial with either Unlocated or LocatedFuzzy as Unverified. If the renderer follows this text, a supported fuzzy-only quote can be counted/rendered with ordinary supported findings rather than in the non-grounded split (or a separate fuzzy/caution treatment), understating the evidence caveat the report carries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed (LocatedFuzzy at verify.rs:136; is_groundable_support accepts exact Located only) and fixed in 92ca05b: the non-exact-grounding treatment now covers Unlocated and LocatedFuzzy alike — Supported with either renders in the unconfirmed-supports sublist annotated with which, Partial with either stays annotated in the disagreements bucket, and the DoD matrix covers all four verdict×status combinations.
Generated by Claude Code
…ywhere, LocatedFuzzy The quote requirement scopes to findings carrying passage: NotSupported is typically a no-quote verdict (NotApplicable) whose line states no supporting passage was found, and source_excerpt only renders as labeled context. archive_of renders on any line carrying it — the contract stamps it on archive-backed disagreements too, which keep their repair handle in the disagreements bucket while recovered-via-archive narrows to Supported. LocatedFuzzy joins Unlocated in the non-exact-grounding treatment, matching the exact-Located-only grounding gate, with the DoD matrix covering all four verdict-by-status combinations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8309366d58
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - [ ] Quoted evidence containing wikitext markup (templates, refs, links) is | ||
| `<nowiki>`-escaped so the appendix never transcludes or breaks page | ||
| markup, verified by a malicious-quote fixture. |
There was a problem hiding this comment.
Because quoted evidence is arbitrary source text, a quote containing a literal </nowiki> closes a naive nowiki wrapper and lets any following {{template}}, <ref>, or link markup execute when pasted. This DoD only requires fixtures with templates/refs/links, so an implementation can pass while still being vulnerable to the terminator case; please require the escaping helper or malicious fixture to handle </nowiki> specifically.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the classic wrapper-terminator breakout, and the fixture requirement would indeed have let a vulnerable implementation pass. Fixed in 67d7c3f: the escaping helper entity-encodes nowiki terminators (and markup-significant angle brackets) inside quoted content rather than trusting the wrapper, and the DoD's malicious-quote fixture now includes a literal </nowiki> with the assertion that markup following it renders inert.
Generated by Claude Code
There was a problem hiding this comment.
(Replying on the round-5 finding about unusable_reason here is wrong thread — see the dedicated reply on that comment.)
Generated by Claude Code
A quote containing a literal </nowiki> closes a naive wrapper and lets following markup execute; the escaping helper entity-encodes terminators inside quoted content and the malicious-quote fixture must assert the breakout case renders inert. Codex round 4 on PR #129. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67d7c3ff71
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 4. *Unreadable sources* — fetched but not machine-readable (PDF, viewer | ||
| shells), honestly framed as a tool limitation: the citation may be fine. |
There was a problem hiding this comment.
For SourceUnavailableReason::Unusable findings, the report can carry CitationFinding.unusable_reason (PdfBody, ViewerShell, NavChromePaywall, etc.), and the existing page renderer already surfaces that detail so reviewers can distinguish a PDF from a viewer shell or paywall. This bucket only requires generic unreadable-source wording, and the DoD never asserts the specific reason, so an implementation can pass while dropping a report disclosure despite the earlier “every disclosure renders” invariant; require each unreadable line to include a reader-facing reason when the field is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — unusable_reason is a report disclosure and the existing page renderer already surfaces it, so the generic wording quietly violated the every-disclosure rule. Fixed in 4f9388f: each unreadable line renders the specific reader-facing reason when the field is present (PDF vs viewer shell vs paywall, via the copy module — the reviewer's next move differs by which), with a DoD item over fixtures carrying distinct reasons. (Apologies for the stray reply on the round-4 thread pointing here.)
Generated by Claude Code
The field is a report disclosure (PDF vs viewer shell vs paywall) and the reviewer's next move differs by which; dropping it violated the every-disclosure-renders rule. Codex round 5 on PR #129. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f9388f410
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 2. *Recovered via archive* — findings whose claim was **supported** through | ||
| an archive fallback: the live URL is dead and `archive_of` is the repair | ||
| handle — "update the citation to the archive." Pulled out of the | ||
| supported list because they are actionable. (The contract stamps | ||
| `archive_of` on any non-unavailable archive verdict, including archive- |
There was a problem hiding this comment.
For an archive fallback whose panel verdict is Supported but whose grounding is Unlocated or LocatedFuzzy, this bucket and the later unconfirmed-supports bucket both match. Fresh evidence from the code is that try_archive_fallback stamps archive_of on any non-SourceUnavailable archive verdict (crates/sp42-citation/src/citation/page.rs:223-227), while the grounding contract explicitly allows Supported with non-exact grounding (crates/sp42-citation/src/citation/verify.rs:117-139); a renderer following these bullets can therefore either hide the grounding caveat under “recovered via archive” or duplicate the finding despite the exactly-one-sublist invariant. Please define the precedence and test that combined case.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real collision — fixed in a18d9df with the same shape as the round-2 precedence rule, extended into the Supported buckets: grounding exactness partitions, archive_of annotates. An archive-backed Supported with Unlocated/LocatedFuzzy grounding renders in the unconfirmed-supports sublist carrying its repair handle (filing it under "recovered" would overstate the evidence; the handle renders anywhere), recovered-via-archive narrows to Supported+exactly-Located, and the DoD tests both combinations with the never-twice assertion.
Generated by Claude Code
| 2. **CLI surface.** `OutputFormat::GaAppendix` accepted by `verify-page` — | ||
| with an explicit `#[value(name = "ga-appendix")]`, because the enum's | ||
| existing `#[value(rename_all = "lowercase")]` would otherwise expose | ||
| `gaappendix` and break the advertised flag and the CLI tests; new |
There was a problem hiding this comment.
Under the current CLI wiring, FormatArg is the shared --format parser and is flattened into multiple rendering commands (crates/sp42-cli/src/main.rs:260-265, plus verify, verify-page, bare-url preview, and preview args), so adding GaAppendix to OutputFormat here makes Clap accept and advertise --format ga-appendix for non-page-report commands too. Unless those commands really produce a GA appendix, users will get either nonsensical output or late “unsupported format” failures; use a command-local page-report format enum or wrapper so only verify-page and render-report expose this value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — FormatArg is flattened into verify, bare-url preview, and preview as well, so a shared-enum variant would advertise ga-appendix on commands that can't produce one. Fixed in a18d9df: the sketch's CLI phase now specifies a command-local page-report format enum (Text | Json | Markdown | GaAppendix, explicit value name) for verify-page and render-report only.
Generated by Claude Code
…nd-local format enum Within Supported, grounding exactness partitions and archive_of annotates: an archive-backed Supported with non-exact grounding renders in unconfirmed supports with its repair handle, never in recovered-via-archive and never twice, with both combinations in the DoD. The CLI phase moves ga-appendix off the shared OutputFormat (flattened into unrelated commands) onto a command-local page-report format enum for verify-page and render-report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18d9df104
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - [ ] Quoted evidence containing wikitext markup (templates, refs, links, | ||
| **and a literal `</nowiki>` terminator**) is escaped so the appendix | ||
| never transcludes or breaks page markup — the terminator case asserts | ||
| that markup *following* an embedded `</nowiki>` still renders inert — | ||
| verified by a malicious-quote fixture covering all four shapes. |
There was a problem hiding this comment.
When a rendered claim or source_excerpt contains literal wikitext (for example an article sentence discussing {{Infobox}} or source text containing </nowiki>), this acceptance item still passes because it only requires the malicious fixture for quoted evidence. The renderer also outputs claims and optional context above, and the implementation sketch says every verbatim field should be escaped, so please bind the PRD/DoD to escaping those fields too; otherwise a pasted appendix can still transclude or break markup outside passage.quote.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right — claims are article wikitext by construction and source_excerpt is arbitrary source text, so scoping the fixture to quotes left both unbound (the sketch said "every verbatim field" but the PRD's DoD didn't). Fixed in 1ec3ffe: the escaping rule and DoD item now bind every rendered verbatim field, with the malicious fixture carrying hostile content in each field kind (a claim discussing {{Infobox}}, an excerpt embedding a </nowiki> terminator, a quote with a ref tag).
Generated by Claude Code
Claims are article wikitext by construction and source_excerpt is arbitrary source text; the DoD's malicious fixture now carries hostile content in each field kind. Codex round 7 on PR #129. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ec3ffe50b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 6. *Supported findings* — a compact one-line-each spot-check record | ||
| (ref label, claim prefix, grounding marker); the reviewing guide | ||
| expects the reviewer to say what they checked, and counts alone are not a | ||
| record. Quotes stay in the CLI/structured rendering. |
There was a problem hiding this comment.
When a single ref bundles multiple cited URLs, extract_use_sites emits one finding per URL while cloning the same ref_id (crates/sp42-citation/src/citation/extract.rs:119-132). In that scenario two supported entries can share the same derived ref label and claim prefix, so the proposed compact line cannot tell reviewers which cited source was spot-checked; require the provenance/source link or another per-use-site discriminator here as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — bundled refs clone ref_id across per-URL use-sites (ADR-0011's one-use-site-per-cited-source decision), so ref label + claim prefix alone can be ambiguous. Fixed in 8f1134c: the supported line gains the source link as a required discriminator (load-bearing, not decoration), with a bundled-ref fixture in the DoD asserting two same-ref_id supported lines render distinguishably.
Generated by Claude Code
| - [ ] No raw contract identifiers (`NotSupported`, `SourceUnavailable`, enum | ||
| variant names generally, and raw `cite_ref-…` ids) appear in the | ||
| appendix; all verdict/status vocabulary comes from the reader-facing | ||
| copy module and ref labels are derived, verified by a renderer | ||
| assertion scanning output over a fixture exercising every verdict and |
There was a problem hiding this comment.
When extraction or verification fails, the saved report can carry raw cite IDs inside BlockFailure.reason (extract.rs:106-110, page.rs:471-477), and the renderer also must render failures as first-class lists. Because this acceptance check only scans a fixture exercising verdicts/statuses, a renderer can pass while leaking cite_ref-… from failure text; include skipped/extraction-failure records in this no-raw-id fixture or require the renderer to rewrite those reasons.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed — BlockFailure.reason embeds raw cite ids (extract.rs formats ref {ref_id} has no resolvable claim text). Fixed in 8f1134c: failure and skip reasons render through the copy module with embedded cite ids rewritten to derived labels, and the no-raw-identifiers DoD fixture now includes skipped and extraction-failure records.
Generated by Claude Code
…n covers failure reasons A bundled ref emits one finding per URL sharing the same ref_id, so the compact supported line requires the source link to say which source was spot-checked; and BlockFailure.reason strings embed raw cite ids today, so failure/skip lines render through the copy module with ids rewritten and join the no-raw-identifiers fixture. Codex round 8 on PR #129. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f1134ce32
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| findings render one line each (derived reader-facing ref label, claim | ||
| prefix, grounding marker), |
There was a problem hiding this comment.
Fresh evidence after the bundled-ref fix is that the main section and DoD now require the source link as the per-URL discriminator, but this resolved-question summary still says supported lines contain only the ref label, claim prefix, and grounding marker. If implementers follow this decision summary, bundled refs with multiple URLs can again produce indistinguishable supported spot-check lines, so the summary should include the source link too.
Useful? React with 👍 / 👎.
| a no-raw-contract-identifiers scan), nowiki escaping over a | ||
| malicious-quote fixture, determinism with a pinned timestamp. |
There was a problem hiding this comment.
Fresh evidence after the verbatim-field escaping fix is that this phase still only calls for a malicious quote fixture. Claims and source_excerpt are rendered verbatim too, and the PRD now requires those fields to handle templates/refs/</nowiki> safely; if this sketch drives implementation, claim or excerpt breakout cases can ship untested while satisfying the phase checklist.
Useful? React with 👍 / 👎.
2 participants
Summary
Docs only — the assessment domain's second PRD and its implementation sketch, following on from #124.
docs/domains/assessment/prd/0016-ga-evidence-appendix-renderer.md(Discussion) — a pure renderer from the existing report contracts (PageVerificationReport, ADR-0011;StabilitySignalstaged behind PRD-0015) to the pasteableTalk:Article/GAnwikitext evidence appendix: the GA review-assist design sketch's first build step and the futuresp42-assessmentcrate's first code. All three open questions were resolved with the Editor, then the document went through a full paragraph-by-paragraph review. What the review settled:supported_unlocatedcounter noted upstream for the references domain), and the report carries no run timestamp (determinism pinned via shell-injectedrendered_at; additiveverified_atnoted upstream){{GAList}}native-idiom question explicitly routed to alpha copy review with real GA reviewersdocs/design-plans/2026-07-10-ga-appendix-renderer.md— the implementation sketch, written in parallel with the PRD and grounded in the tree (it corrected the PRD three times before review even started: grounding axis, staged stability DoD, saved-report mode as the core surface). Four phases: crate + pure builder, CLI surfaces (render-report+ga-appendixformat), fixture realism with a sandbox paste, and a staged stability phase landing with PRD-0015's implementation. Synced with every review-time change.Type
Validation Required
Docs-only change — no code paths touched. Cross-references checked against the tree (
PageVerificationReport/CitationFindingfields,ReportDocumentshape, CLIOutputFormat, workspace layout) while writing the implementation sketch.Risk
No ADR: the renderer is pure composition over existing contracts, and the new
sp42-assessmentcrate follows ADR-0013 via theadding-a-domain.mdturnkey path (rationale recorded in the PRD header).Notes For Reviewers
Discussion: resolved questions carry the Editor's decided answers, open to reviewer reaction until acceptance.supported_unlocatedonPageVerificationStats, and additiveverified_atonPageVerificationReport— both close gaps the renderer works around today.{{GAList}}deferral.🤖 Generated with Claude Code
https://claude.ai/code/session_01ATEQwdt8J9mVb94EcgRFY3
Generated by Claude Code