Bug-hunt round 28: review id-lookup SafeText mismatch, file-constant reuse, Mode B tense

[REPPL]

Round 28 of the automated bug-hunt loop (state: #24).

Confirmed findings (fixed)

1. internal/review/review.go:112-119 (findByID), :509-511 (contains) — substantive
Both compared a finding's ID with raw ==, while a finding's id is only ever displayed through session.SafeText (internal/report/report.go, internal/review/review.go's printFinding). analyze.Load/analyze.ParseRecords never re-validates ^F-\d{3}$ on load — only analyze -ingest does — so a hand-edited or exchanged findings.jsonl can carry a raw id with an invisible character (e.g. a zero-width space) that displays as a clean id like F-001 everywhere but was unreachable by that same id: testimony review -finding F-001 -verdict confirmed failed with finding F-001 not found, and the same mismatch blocked marking any finding a duplicate-of it.

Deeper still: even resolving the lookup, single() (non-interactive) and the interactive duplicate-of path recorded the verdict under the operator's clean flag/typed value rather than the finding's actual raw id. analyze.EffectiveStatus keys its map on each finding's raw id, so a verdict recorded under the clean form would silently fail to attach — the finding would keep showing "unverified" in report.md even though a verdict was successfully appended to findings.jsonl.

Fix: findByID/contains now compare ids in their session.SafeText rendered form (matching the load-time uniqueness check analyze.ParseRecords already applies — its own comment states id-keyed consumers assume this). Verdict recording now stores the resolved finding's actual raw id for Finding, and the resolved duplicate target's raw id for Of, in both the non-interactive and interactive paths. Covered by a new regression test (TestNonInteractiveConfirmMatchesRenderedID) that writes a finding with a raw zero-width-space id and asserts the verdict attaches under analyze.EffectiveStatus; verified failing before the fix (finding F-001 not found) and passing after.

2. internal/cli/cli.go:98,126 — nitpick
The merge/report success messages and report's write path named timeline.jsonl/report.md as independent string literals instead of session.TimelineFile/session.ReportFile, the constants already used for the same filenames elsewhere (internal/timeline/timeline.go, tests). Same values today, but two independent spellings of one well-known name is exactly what the constants exist to prevent. No behavioural change, so no new test.

3. .abcd/development/brief/01-product/01-purpose.md:26-28 — nitpick
Described Mode B's keyframe extraction ("keyframes extracted from the video supply the referents") in unqualified present tense, as if shipped, while 04-analysis.md:46 heads that section "Keyframes (planned)" and defers frame extraction to a later intent, and 01-phases.md:9 marks Phase 4 (Mode B) "Not started". The same page already qualifies its other unshipped item ("codebase mapping as a future goal") one paragraph earlier — this brings the Mode B mention in line with that existing convention.

Considered and rejected (adversarially refuted, or precedent duplicates)

• report.go's orDash fallback branch is unreachable given eventLine's current call order — true, but a deliberate locally-redundant guard against a future caller-invariant change, matching the same rationale review.go's checkTargets states for its own belt-and-braces SafeText calls.
• demo's -addr ":" (empty port) bypassing CheckAddr's numeric range guard — not silent (the real bound ephemeral port is reported back), and an empty port is net.Listen's own deferred-to-runtime case, the same as a named service port; this guard's scope was already settled by an earlier round's identical precedent.
• itd-2-analysis-findings.md's AC2 wording ("the finding's status becomes...") read as contradicting the append-only invariant — AC2's own second clause names the append-only mechanism; "status" at intent altitude is the effective status the pipeline derives and displays, not the stored field the invariant docs constrain.
• 02-verification.md's "kept under sessions/" line read as unverifiable — sessions/ is gitignored by design; committing a real captured session would violate the repo's own privacy rule.
• Excluded before verification, as precedent duplicates of findings already discussed in earlier rounds: AGENTS.md's dangling 03-configuration.md link inside the abcd-managed fence; AGENTS.md claiming CI runs plain go test ./...; persona role-label wording drift across intent drafts; the intents-README "always they/them" persona-quote rule read against the personas page's gendered narrative pronouns.

Verification

go build, gofmt -l ., go vet ./..., go test ./..., go test -race ./..., and the pipeline smoke (merge/report on examples/sample-session) all pass.

.abcd/work/DECISIONS.md gains the round's entry.