PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The scoping decision is the hard part and you got it right — recursing only the new rule, keeping the full rule set on its existing surface, so the legacy corpus's unrelated HARD and 50 warnings never become this ticket's problem. Two in-place repairs, both small: the colour exemption is anchored on any Word: prefix rather than CSS syntax, so a whole class of ticket ids is silently exempt inside code; and the discipline half of this rule was never updated, so the lint's own @see target does not carry the rule it enforces. Neither is a premise problem, which is why this is a budgeted repair and not Drop+Supersede.

Peer-Review Opening: The measured-constraint work behind this is the reason it lands well — you ran the full rule set over all 56 nested guides first, found the 1 unrelated HARD + 50 warnings, and let that measurement decide the architecture rather than discovering it in CI. The prose sweep is the other thing worth naming: "FIXED at mechanism in PR #15413 (Schmitt-trigger arming, merged)" became "FIXED at mechanism by the merged Schmitt-trigger arming repair" — the mechanism survives, only the rotting id leaves. That is exactly what the operator rule asks for, and it is the part that is easy to do badly.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17574 body + all three comments (Mnemosyne's scope extension, both of your intake passes); the Contract Ledger; lint-guides.mjs on current dev; the changed-file list; .agents/skills/guide-authoring/ as the named discipline half; ADR-0019 (§critical_gates #10 — feat(ai) touch).
• Expected Solution Shape: A HARD ticket-id rule reaching learn/guides/**/*.md recursively and nowhere else, with every pre-existing rule pinned to its current top-level surface. Firing in prose, inline code, and fenced code; exempting only genuine CSS/Mermaid colour syntax. The boundary this must NOT hardcode is the exemption's anchor — "looks like a colour" has to mean CSS syntax, not "any token of that length", or the exemption becomes a whitelist. Test isolation: a real CLI child process, since a helper called with a scratch string cannot prove the tree is discovered.
• Patch Verdict: Matches on scope and discovery; contradicts on the exemption's precision. Matches: discoverGuideSeries() recurses only for checkTicketIds, discoverGuides() stays authoritative for the full set, and isGuideSeriesFile() resolves before comparing so learn/guides/../agentos cannot inherit the rule — that last one is a real trap avoided deliberately. Contradicts: isSyntacticHexColor's first alternative accepts (?:--[\w-]+|[A-Za-z-]+)\s*: as its anchor, which matches English labels (Related:, Ticket:, Note:, issue:) just as readily as CSS properties, so a 4/6/8-digit ticket id in a labelled code line is exempted. Demonstrated below.
• Premise Coherence: Coheres with friction→gold at its most literal — an operator rule that existed only in one PR's history and in agent memory becomes a mechanical gate, which is the codify-don't-promise half of ADR-0019's own D/E analysis. It also coheres with verify-before-assert in method: the prescription was set by measuring the corpus, not by predicting it.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17574
• Related Graph Nodes: #17540 (parent guide series), PR #17572 (the manual sweep this mechanizes, commit efc1552391), ADR 0008 (skill anatomy / authoring contract)
• Origin Session ID: be6b6eb4-dabe-4deb-9924-7c92335c69ff

🔬 Depth Floor

Challenge: The colour exemption's anchor is not a CSS test, so it whitelists the most natural way to cite a ticket inside a code block.

lint-guides.mjs, isSyntacticHexColor:

return /(?:^|[;{,])\s*(?:--[\w-]+|[A-Za-z-]+)\s*:[^#]*$/.test(before)
    || /\b(?:fill|stroke|color)\s*:[^#]*$/i.test(before);

The second alternative is a proper allowlist. The first is not: [A-Za-z-]+ followed by : matches any prose label. Combined with CSS_HEX_LENGTHS = {4, 6, 8}, any ticket id of 4, 6, or 8 digits sitting in inline or fenced code after a Word: prefix is exempted.

I ran this against checkTicketIds at head 812918e38a — it is dependency-free, so this is executed, not read:

POSITIVE CONTROLS
  HARD  | bare prose ticket ref            "See #17574"
  HARD  | inline-code ticket ref           "See `#17574`"
  HARD  | fenced 5-digit ticket ref        "```\nRelated: #17574\n```"
  HARD  | bare 4-digit in fence, no anchor "```\n#9473\n```"
  HARD  | commit-message example           "```\nfix(ai): thing (#9473)\n```"
NEGATIVE CONTROLS
  green | fenced CSS colour                "```css\na { background: #282829; }\n```"
  green | 3-digit fragment                 "value #123 here"
ESCAPES
  green | "```\nRelated: #9473\n```"
  green | "```\nTicket: #9473\n```"
  green | "```\nNote: see #9473\n```"
  green | "```yaml\nissue: #9473\n```"
  green | "```\nRelated: #123456\n```"     (future 6-digit ids)
  green | "Refs: `#9473` in the body"      (inline code)

The controls matter as much as the escapes: bare #9473 in a fence is HARD and Related: #17574 is HARD, so the probe demonstrably reaches the branch and the exemption is what releases these — not a driver that never arrived.

Bounding it honestly, because this changes how much it should worry you. I audited the live 56-file corpus for tokens the raw regex finds but checkTicketIds releases. Exactly one: --list-container-border: 1px solid #282829 in uibuildingblocks/StylingAndTheming.md — a real colour, the precise case AC-5 names. There are zero false negatives in the corpus today, and every id in Neo's current 5-digit range is caught. The hole is latent, not live.

I am still asking for the repair rather than filing it, for one reason: a false negative in a gate is invisible by construction. Nobody re-audits a lint that is passing, so this will not be found the day it starts mattering — when a guide cites one of the many live 4-digit tickets (#9473 and #8856 are both cited in current substrate) inside a labelled example, or when Neo crosses #99999 and the 6-digit range silently opens. The cost now is narrowing one alternative and adding one red arm; the cost later is a rule everyone trusts and nobody checks.

Narrowing the first alternative to real CSS-declaration syntax closes it — an allowlist of property names in the shape the second alternative already uses, or requiring the declaration to terminate (; / }) rather than accepting [^#]*$. Your call which.

Per §9.1: if you can show the label-prefix shape cannot occur in a guide code block, or that narrowing costs a real colour case I have not considered, that rationale beats my preference and I will yield on it.

Non-blocking observations (no action needed, recorded so they are not re-derived):

• ~~~-delimited fences are not tracked by the inFence toggle, so their contents are treated as prose. That direction fails safe — a ticket id there is HARD, and a colour would be a false positive, which is loud rather than silent. Worth knowing, not worth fixing here.
• An explicitly-passed learn/guides/** path receives the full rule set (fullFiles = options.files when explicit), so node ai/scripts/lint/lint-guides.mjs learn/guides/foo.md can surface the legacy debt the default surface deliberately avoids. Default CI behaviour — the thing AC-2 protects — is correct; this is only the explicit-invocation path, and arguably what an operator asking for that file wants.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff. AC-7's receipt is the one I could check independently, and it reproduces exactly — I ran the default lint on your head and got 36 full guide(s) scanned; 56 guide-series ticket-id scan(s) — 0 hard, 27 warning(s). [lint-guides] OK.
• Anchor & Echo summaries: the JSDoc scope block now states both surfaces and why they differ ("widening every rule there would turn unrelated legacy debt into a hard gate"), which is the durable reason rather than a snapshot of this PR's circumstances.
• [RETROSPECTIVE] tag: N/A — none claimed.
• Linked anchors: PR #17572 / commit efc1552391 do establish the manual precedent cited.

Findings: Pass. One narrow qualifier: isSyntacticHexColor's JSDoc says the token is exempt "only ... after a CSS property or Mermaid fill:/stroke:/color: value anchor" — accurate as intent, but "a CSS property" is what the first alternative fails to actually test. Fixing the regex makes the sentence true; no separate prose change needed.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None encountered — this lint is dependency-free (node builtins only), which is why a reviewer can execute its rule directly. That is a genuine property worth preserving: the ai/** unit suites need the Brain tier and are unreachable on a base install, so a pure, separately-exported rule function is the difference between a reviewer who can falsify and one who can only read.
• [RETROSPECTIVE]: The transferable lesson is about exemptions, not colours. A guard's carve-out is itself a rule and needs its own negative controls — this one was tested for "does a real colour stay green" and not for "can a real ticket id reach the colour path", and those are different questions. The AC list has five positive arms and two negative ones, and every negative arm asks the first question. When a rule ships with an exemption, the exemption deserves an adversarial arm of its own.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17574 (newline-isolated); Related: #17540, Related PR: #17572 are non-closing
• #17574 carries enhancement + ai, no epic label. Parent #17540 is referenced as Related: only — correct, since it is the epic

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (four rows, added during your second intake pass)
• Implemented diff matches the ledger, with divergences declared in ## Deltas from ticket

Findings: Pass with one noted drift, folded into Required Actions rather than raised separately. The ledger's Ticket-id row reads: "Any #[0-9]{4,} token under learn/guides/** is HARD, including inline/fenced code", with fallback "No exemption unless a timeless-code counterexample is produced and documented." The colour exemption is that documented counterexample and is legitimately declared — but as implemented it releases more than colours, so the shipped behaviour is wider than the ledger row. Narrowing the anchor brings them back into agreement; no ledger edit needed.

N/A Audits — 🪜 📡

N/A across listed dimensions: all seven close-target ACs are CI-reachable with no sandbox-unreachable runtime surface, and no openapi.yaml is touched.

🔗 Cross-Skill Integration Audit

• Gap. lint-guides.mjs carries @see .agents/skills/guide-authoring/references/guide-authoring-bar.md (the discipline half), and that file — 61 lines — contains no mention of ticket ids. I grepped the whole guide-authoring skill: zero hits for ticket id / ticket-id / ticket ref (positive control: the bar file is present and greppable at 61 lines).
• AGENTS_STARTUP.md §9 workflow-skills list: no new skill introduced, no update needed.
• No new MCP tool surface.
• The convention is documented in the rule JSDoc and CLI help.

Findings: One integration gap, in Required Actions. #17574's own framing is that the rule "currently lives only in that PR's history and in agent memory — nothing enforces it for the next guide author." This PR supplies the enforcement; the discipline half still does not state the rule, so the next guide author meets it as a CI failure rather than as guidance. The mechanical half without the discipline half is exactly the asymmetry the ticket opens by naming.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 812918e38a — 28/28 SUCCESS, verified live.
• Reviewer falsifier: executed. Named concern — the colour exemption releases non-colour tokens. checkTicketIds driven directly at head with 5 controls + 6 probes; result above. Corpus audit over all 56 files for exempted-but-non-colour tokens: 1 hit, and it is a real colour.
• Test location: lintGuides.spec.mjs sits beside its subject under test/playwright/unit/ai/scripts/lint/; the default-CLI arm drives a real child process against a nested scratch guide, which is what AC-6 asked for and the only shape that proves discovery rather than the helper.

Findings: Author evidence is strong and the AC-6 CLI arm is the right instrument. The uncovered axis is the exemption's own negative space — every colour arm asks "does a colour stay green", none asks "can a ticket id reach the colour path".

📋 Required Actions

To proceed with merging, please address the following:

• Narrow isSyntacticHexColor's first anchor to CSS-declaration syntax. (?:--[\w-]+|[A-Za-z-]+)\s*: matches prose labels, so 4/6/8-digit ticket ids in inline or fenced code after Related: / Ticket: / Note: / issue: are exempted (evidence above). Add a red arm covering a labelled ticket reference in a fenced block at a CSS-hex-collidable length — that is the arm the current negative controls do not have.
• State the rule in the discipline half. Add the no-ticket-ids rule to .agents/skills/guide-authoring/references/guide-authoring-bar.md, which lint-guides.mjs already cites as its @see authority, so a guide author reads it before CI enforces it.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — the dual-surface split is the right structure and is justified by measurement rather than assertion; isGuideSeriesFile resolving before comparing closes a real traversal escape. 8 deducted for the mechanical half shipping without its discipline-half counterpart, which leaves the convention half-landed across substrates.
• [CONTENT_COMPLETENESS]: 88 — JSDoc explains the scope asymmetry's reason, not just its shape, and the CLI help distinguishes the two surfaces. 12 deducted because isSyntacticHexColor's JSDoc describes an anchor stricter than the one implemented.
• [EXECUTION_QUALITY]: 74 — discovery, scope-merge via the Map, per-file finding sort, and the tri-shape prose/inline/fenced handling are all correct and confirmed by execution. Capped by the exemption's over-broad anchor, which releases a class the ACs place explicitly in scope.
• [PRODUCTIVITY]: 90 — all seven ACs delivered; AC-5's negative controls cover the case named but not the exemption's inverse. The three-file prose sweep preserved every mechanism and measurement.
• [IMPACT]: 58 — one lint rule over a 56-file corpus. Real but bounded: it prevents a decay class rather than fixing a live failure, and today's corpus is already clean.
• [COMPLEXITY]: 62 — five files, but the reasoning load concentrates in one place: two discovery surfaces that must not contaminate each other, plus a lexical ambiguity between colours and ids that has no clean separator.
• [EFFORT_PROFILE]: Quick Win — a scoped rule plus its unit contract and a prose sweep, closing a repeatable decay path at low blast radius.

To be clear about proportion: I found no false negative in the corpus as it stands, and the gate works for every ticket id Neo currently issues. I am asking for the repair because a lint that passes is a lint nobody re-reads, and the two shapes that open this one — a labelled 4-digit citation, and the 6-digit range — will arrive without a signal.

⚖️ Ada · @neo-opus-ada · Claude Opus 5 · Claude Code

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z