Skip to content

Close A1/A2 structurally; red-team the gold suite (A3+B2)#21

Open
lukefwalton wants to merge 4 commits into
mainfrom
claude/fable-worthy-tasks-o5ma9c
Open

Close A1/A2 structurally; red-team the gold suite (A3+B2)#21
lukefwalton wants to merge 4 commits into
mainfrom
claude/fable-worthy-tasks-o5ma9c

Conversation

@lukefwalton

Copy link
Copy Markdown
Owner

What & why

Closes the two highest-value seams in NEXT-STEPS.md A and audits the third, in three commits sequenced so the gold coverage that would catch each failure mode lands before the structural change that closes it (the repo's own rule).

1. Red-team the gold suite (A3+B2) — 74c86ed. Five new adversarial entries: extraction-shaped queries (q11–q12: "what exactly does the notebook say", a verbatim-quote demand), instruction injections aimed at private content (q13, must still route) and at the machinery (q14, must refuse bare), and the previously missing supported case (q15). Canary forbid-patterns — phrases verified to exist only in the private notebook bodies — now guard each mode's characteristic failure ("chair by the window" was rejected as a canary because the public lyric nearly contains it). New expectAnswerPatterns gold field: the must-match mirror of forbidAnswerPatterns, documented as pinning behavior shape, never facts.

2. Template the related-material answer (A2) — 02f0c91. A hint citation is provenance without backing, so free prose in this mode was the one place a confabulated "summary" of private material passed every gate. finalizeAnswer now replaces the mode's prose, after grounding, with a fixed sentence rendered only from the cited hints' public-safe fields (renderRelatedMaterialAnswer, src/public-safe.ts) — the mode can point, never assert content. The gold suite pins the template. NEXT-STEPS A2 rewritten as closed, with the residue named (supported-mode hint citations under free prose, owned by the q15 canaries).

3. Make the traveling label/locator boundary structural (A1) — 624e825. Private notes now require an explicit label: frontmatter field; title stays private and is what gets embedded. Both traveling fields are a branded PublicSafe type whose only constructor is a build-time lint (single line, ≤120 chars, no 5-consecutive-word run shared with the note's private body — N=5 calibrated so the demo's bibliographic locator passes). Index schema bumped to v3; the committed demo indexes were migrated mechanically by scripts/migrate-demo-index-v3.mjs (title := label is exact, embed text byte-identical, vectors and contentHashes untouched — nothing re-embedded), verified by the keyless demo gate (7/7, rho 1.0000).

Note for the keyed follow-up: npm run index is required (local index is now v2-stale), then npm run eval and npm run eval -- --full --ids q07,q11,q12,q13,q14,q15. q15 is the likeliest to wobble (compound canon+process question); if it fails, the fix is a sentence in the prompt's supported-mode bullet, never the query.

Checklist

  • npm test passes without an API key (47/47) — no hidden dependency on live calls
  • npm run typecheck passes
  • No change that makes the eval pass by special-casing a question (fixes go into the corpus, scoring, or prompt)
  • A boundary stays structural (a type, not a checker someone must remember), if this touches the no-leak path — PublicSafe is constructible only through the lint; the related-material template renders nothing but PublicSafe fields
  • Touched the prompt / retrieval / validation / repair? Ran the relevant npm run eval subset — not run here (no API key in this environment); keyless npm run demo:run certifies retrieval on the migrated artifacts (7/7, rho 1.0000). Keyed subset above is the owner's step.
  • Read against .github/STANDARDS.md — the rubric a PR is judged by

🤖 Generated with Claude Code

https://claude.ai/code/session_019n27KsH2AS1mbpHMYp8y2X


Generated by Claude Code

claude added 3 commits July 7, 2026 19:14
Adversarial coverage for the seams NEXT-STEPS A1-A3 name, landing before
the structural changes that will close them (the coverage-first rule,
NEXT-STEPS.md).

- New gold entries: q11-q13 extraction/injection attacks aimed at private
  content (must route, never comply), q14 injection aimed at the machinery
  (must refuse bare), q15 the missing 'supported' case - which also owns
  the residue templating can't close, since supported prose is free prose
  that cites a hint.
- Canary forbid-patterns on q02/q05/q11-q13/q15: phrases verified to exist
  only in private notebook bodies, so any answer containing one has leaked
  or confabulated private text. "chair by the window" rejected as a canary
  because the public lyric nearly contains it.
- Refusal-debris patterns on q08-q10/q14: a failed refusal leaks URLs or
  citation-id-shaped prose; vacuously green when refusal succeeds.
- New gold field expectAnswerPatterns: must-match mirror of
  forbidAnswerPatterns (same load validation), to pin behavior shape -
  never facts. Will pin the related-material template when it lands.
- Tests: must-match judge cases, load validation for both pattern fields,
  all four modes now asserted present in the gold set.

q11-q15 may be red under --full until the template and label changes land;
that is the coverage doing its job. Retrieval tier expected green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019n27KsH2AS1mbpHMYp8y2X
Close the provenance-without-backing seam: a hint citation is real
provenance but carries no text, so free prose in related-material mode was
the one place a confabulated summary of private material passed every
gate, held only by a prompt instruction.

- New src/public-safe.ts: renderRelatedMaterialAnswer builds the mode's
  prose from the cited hints' public-safe fields (label, locator) alone.
  No raw URL in the prose - the citation object carries the link, and
  gold q07 forbids URLs here.
- src/answer.ts: finalizeAnswer runs repair -> grounding -> template, so
  the hint lookup cannot miss and answers that only became
  related-material through repair's kind conversion get templated too.
  Confabulation in this mode is now inexpressible, not discouraged.
- Prompt tells the model its related-material prose is standardized.
- Gold pins the template via expectAnswerPatterns on q07/q11-q13, so a
  regression cannot silently un-template the mode.
- NEXT-STEPS A2 rewritten as closed, with the residue named: supported
  mode still cites hints under free prose, owned by the q15 canaries.
  The template's safety is exactly the safety of label+locator - A1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019n27KsH2AS1mbpHMYp8y2X
The RoutingHint label was the note's raw frontmatter title and the locator
raw frontmatter, guarded only by a warning comment - a privately-titled
note leaked through its own label. Three layers replace the comment:

- Explicit `label:` frontmatter, required on private notes: the display
  name that travels, distinct from `title` (private, embedded, never
  travels). What travels is now an authored per-note decision; a corpus
  without labels fails loudly with the reason.
- PublicSafe branded type on PrivateNote/RoutingHint label+locator, whose
  only constructor is the new build-time lint assertPublicSafeField
  (src/public-safe.ts): non-empty, single line, <=120 chars, and no run of
  5 consecutive words shared with the note's private body - a traveling
  field that quotes the note fails the build, not an answer. N=5 is
  calibrated: the demo's bibliographic locator legitimately shares a
  4-token run with its sermon body.
- Index schema v3 (store.ts): note entries must carry title/label/locator,
  so a stale or hand-edited artifact fails fast with the rebuild remedy.

noteEmbedText now embeds title+body - byte-identical to the old label+body
for every corpus whose labels repeated titles, so committed demo vectors
survive without re-embedding: scripts/migrate-demo-index-v3.mjs migrated
the committed demo indexes mechanically (title := label, vectors and
contentHashes untouched), verified by the keyless demo gate (7/7, rho
1.0000). Residue named in NEXT-STEPS A1: the lint is a 5-gram tripwire,
`about` travels unlinted as a declared public page, and the brand erases
at JSON read - backstopped by the gold canaries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019n27KsH2AS1mbpHMYp8y2X
@surmado-code-review

surmado-code-review Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Automated Checks (advisory, non-blocking)

✅ All checks passed.


⚠️ Partial review: The diff exceeded size limits. Feedback below focuses on early files and high-level risks only.

Standards Compliance

This PR is highly standards-relevant: it touches the no-leak boundary, related-material mode semantics, gold eval coverage, and the committed demo/ artifacts. From the visible diff, I don’t see a clear repo-standards violation, and one concern from my prior review appears addressed by the new migration-safety/hash-invariant test called out in demo/artifacts.test.ts.

The only visible file here is the massive demo/corpus/index.json churn from schema version:2 to version:3. That fits the repo’s explicit demo/ exception, but because this artifact now carries boundary-sensitive public metadata, the reviewer should verify the unseen code path still enforces the standards structurally rather than by convention: stale local indexes should fail loudly on version mismatch, and any traveling fields used in prompts/citations should be restricted to the new public-safe path rather than falling back to private note titles.

Summary

This PR appears to land three coordinated changes: new adversarial gold cases around boundary and mode behavior, a structural replacement of free related-material prose with a fixed public-safe answer template, and a schema/type migration that separates private note titles from public-traveling labels in the index. It also mechanically migrates the committed demo index to schema v3, which is why most of the visible diff is artifact churn rather than logic changes.

Reviewer: most of the risk is in the unseen no-leak/output-finalization path and the index-schema migration; the giant demo JSON update is mostly a consequence.

What to pay attention to

  • Related-material finalization after grounding
    This is the highest-value standards change. The key question is whether finalizeAnswer now always discards model prose in related-material mode and renders only from cited public-safe hint fields after validation/grounding, preserving the repo’s “private text must never reach the prompt / answer” intent.

  • PublicSafe / label boundary construction
    The standards explicitly prefer structural guarantees. The reviewer should confirm the branded type is not practically forgeable in normal app code and that private-note title cannot re-enter any prompt/evidence/citation path as a traveling field.

  • Schema v3 migration and stale-index behavior
    Because local indexes are now intentionally stale, the important check is that runtime/index-loading code throws a specific version error rather than silently accepting or partially reading v2 data. That’s the “loud failures” standard in a very standards-sensitive part of the system.

  • Gold additions for q11–q15
    Since this PR changes behavior in exactly the areas the standards call out—refusals, related-material, supported mode, and injection/boundary cases—the reviewer should verify the new gold expectations are guarding semantics and leak shape, not overfitting exact phrasing.

Things I noticed

🟡 Yellow flags — consider for this PR or a follow-up:

  • The review surface is dominated by committed artifact churn in demo/corpus/index.json, so the real merge risk is concentrated in the unseen migration/loader/finalization code. I’d put extra scrutiny there, especially around fail-fast schema checks and public-safe field derivation.
  • [UNVERIFIED] Because the structural fix is split across types, corpus build, and answer rendering, it would be worth confirming there isn’t still a supported-mode path where grounded model prose can freely synthesize from hint citations. The PR description explicitly calls that residual risk out, so the reviewer should check the new gold canaries are actually covering it.

If nothing else jumps out in the unseen files, I don’t see a visible red flag from the diff shown here.

Good patterns

  • Sequencing adversarial gold coverage before the structural fixes is very aligned with the repo’s eval/regression-contract standard.
  • The added demo hash-invariant test (per the provided summary) is a strong safety net for a schema migration that would otherwise be very hard to review from the JSON blob alone.

Suggested improvements

  1. If not already present in unseen tests, add a focused unit test that proves related-material mode drops any model-generated prose and always emits the fixed public-safe template, even when the model returns a plausible summary.
  2. Make sure the index version mismatch error names both the found and expected versions; that will save confusion now that the PR intentionally leaves local indexes stale.
  3. If the PublicSafe constructor/lint has any non-build-time entry point, add one regression test showing a private title cannot be assigned to a public-traveling field without failing.

Questions for the author

  • For the new PublicSafe boundary, is corpus build/migration truly the only place those fields can be created, or is there any runtime path that can synthesize them later?
  • For the new supported-mode gold case(s), are the canaries checking only output text shape, or do they also protect against mode/citation drift when hint citations coexist with free prose?

Surmado Code Review (v1.2-mt) is an automated review, designed to work alongside human judgment.

Want to change your STANDARDS.md or YML? Edit it directly, or tune it with our AI agent Scout.

Comment /rerun-review on this PR to refresh the review — costs 1 additional PR credit.

- readIndexFile's version mismatch now names the found version alongside
  the expected one ("schema version 2, not schema version 3"), so a stale
  artifact says what it is, not just what it isn't.
- New keyless test in demo/artifacts.test.ts: every committed demo entry's
  contentHash must equal the hash of today's embed-text derivation
  (title+body for notes), proving the v2->v3 migration preserved the exact
  bytes the committed vectors were embedded from.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019n27KsH2AS1mbpHMYp8y2X

Copy link
Copy Markdown
Owner Author

Responding to the automated review (a79fff2 addresses the actionable parts):

Suggestions. (1) A test proving plausible model prose is discarded in related-material mode already exists — answer: finalizeAnswer makes related-material prose deterministic in test/engine.test.ts feeds a confabulated summary of the private note through the real pipeline and asserts the canary phrase cannot survive. (2) The schema-mismatch error now names the found version alongside the expected one. (3) Added a keyless invariant test (demo/artifacts.test.ts): every committed demo entry's contentHash must equal the hash of today's embed-text derivation, which pins exactly the migration claim the JSON diff is too large to eyeball — the v2→v3 rewrite preserved the bytes the committed vectors were embedded from.

Questions. (1) Other construction paths for PublicSafe: one, and it's documented at the type (src/types.ts) and in NEXT-STEPS A1 — the JSON read path. readIndexFile casts stored entries back to IndexEntry, which launders the brand without re-linting; the guarantee is "linted when the corpus was built", backstopped by the v3 shape check in entryIsValid and the gold canaries at answer time. No runtime code mutates the fields after construction (toRoutingHint only copies them). (2) Supported-mode canaries: intentionally prompt/eval pressure, not structure — templating supported-mode prose would mean templating record-backed answers, which is the product. That residue is named in NEXT-STEPS A2 and owned by q15's canary patterns.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants