feat(filter/tsc): quarantine unparsed lines (V0.4.1 #5)#57
Merged
Conversation
The tsc parser silently dropped any line that didn't match
DIAGNOSTIC_RE / CONTINUATION_RE / SUMMARY_RE. If tsc 6.x adds a
new line shape, we'd lose 30% of the signal without warning.
ADR `docs/plans/filter-engine/00-decisions.md § 9c-c`
("Quarantine, don't drop") says: surface those lines instead of
swallowing them.
Parser now returns `unparsedLines: { line; text }[]` alongside
`diags` and `stats`. A line is unparsed iff it's non-empty after
ANSI stripping and matches none of the three known shapes.
Orphan continuation (indented line with no current diagnostic)
counts as unparsed — that's a real signal something is off.
Glue (index.ts) builds one `Finding[severity:"info",
ruleId:"unparsed"]` per unparsed line and forwards the line
indices to the renderer. The renderer appends a footer
`⚠ N unparsed line(s) — see combined.log:RANGES` after the
last bullet, with contiguous spans collapsed via
`formatLineRanges` (`[42,50,51,78,79,80]` → `42, 50-51, 78-80`).
Footer also fires on the pass header (zero clusters but
quarantined content) — saying "pass" without flagging the
quarantine would defeat the purpose.
Token-budget lens: the footer earns its tokens by surfacing
a parser failure mode the agent can't otherwise see. The
`unparsed` Findings live in `observation.json` for structured
consumers and don't bloat `filtered.md`.
Bump `tsc/3 → tsc/4`. The rendered shape gains a footer when
unparsed content is present; bumping lets `observation.json`
attribute archived runs to the pre-quarantine renderer.
Existing fixtures (small/medium/large.txt) have zero unparsed
lines — snapshots unchanged.
Verified: 331/331 tests pass (+19 new), biome ci clean,
typecheck clean.
Audit follow-up to fb43c0a (quarantine-don't-drop). Five issues caught between merge candidates. P0 — pretty-mode regression. With quarantine, the tsc filter now emits info-severity Findings for unparsed lines, so a pretty-mode run on a failed exit produces `findings.length > 0`. The dispatcher's own pretty-mode rule (zero findings + non-empty output + non-zero exit → miss) stops firing, and the agent gets `# tsc — pass …\n\n ⚠ N unparsed lines` instead of the raw output. The glue now short-circuits before building Findings: zero real diags + only unparsed lines + non-zero exit → empty view, dispatcher falls back to raw. The genuinely-weird "exit=0 + diags=0 + unparsed>0" branch keeps the pass-header-with-footer shape. P2 — whitespace-only lines were quarantined. `line !== ""` accepts " " and "\t"; classifier now uses `line.trim() === ""` so blank and whitespace-only lines are equivalent. P2 — lone `\r` survived the split. `split(/\r?\n/)` only consumes `\r` paired with `\n`; old-Mac CR endings or partial CRLF left literal `\r` in unparsed text. Normalize once with `replace(/\r\n?/g, "\n")` before splitting on `"\n"`. P3 — `formatLineRanges([5,5])` emitted "5, 5". Callers pass unique inputs today, but the function is exported and tested in isolation, so dedupe via `new Set` defensively. Elegance — extract `classifyLine` (5-variant tagged union) plus `startDiagnostic` / `appendContinuation` helpers. The `parseTscOutputWithStats` body shrinks from 67 LOC to 42 (under the 40-LOC code-line norm once comments and the outer braces are discounted), and the loop reads as a switch on line kind instead of a 4-branch state machine inline. No filter-version bump: rendered shape is unchanged on inputs the old code handled correctly. Pretty-mode runs change from "wrong header" to "raw passthrough" — strictly better, no archived observation will look different because no archived run hit the broken path. Verified: 335/335 tests pass (+4 new — pretty-mode passthrough integration, whitespace-only quarantine, lone-\\r normalization, formatLineRanges dedupe), biome ci clean, typecheck clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
V0.4.1 polish, item #5 of 4. Lands
§ 9c-c"Quarantine, don't drop" fromdocs/plans/filter-engine/00-decisions.md— unparsed lines now surface asFinding[severity:"info", ruleId:"unparsed"]and a footer⚠ N unparsed line(s) — see combined.log:RANGES.Defense against drift
Today the parser silently drops anything it can't recognize. If
tsc 6.xadds a new line shape, we lose 30% of the signal without warning. After this PR, the agent knows what was missed and can drill into raw evidence.unparsedLines: { line: number; text: string }[](1-indexed, in input order).tsc-unparsed-N, titleunparsed: <snippet>).42, 50-51, 78-80).Pretty-mode passthrough preserved
The dispatcher's existing pretty-mode rule (zero findings + non-empty output + non-zero exit →
kind: "miss") was bypassed by the new unparsed Findings (count > 0). Fix is in the glue, not the dispatcher: whendiags.length === 0 && unparsedLines.length > 0 && exitCode !== 0, return an empty view so the dispatcher's existing rule still kicks in. Regression test added.Other audit fixes shipped in the same PR
line.trim() !== "").\rnormalized up-front (text.replace(/\r\n?/g, "\n")); old-Mac CR + partial CRLF both clean.formatLineRangesdedupes input (defensive).parseTscOutputWithStatsrefactored: 67 LOC → 35 (pure code) viaclassifyLinetagged union. Now under the 40-LOC project norm.Filter token test
The footer passes the rule (memory: "filter changes that add tokens must add info Read can't cheaply give"): count + line ranges, no excerpt text. The unparsed text lives only in
Finding.excerpts[](struct consumers), not infiltered.md.Version
tsc/3 → tsc/4. Footer is a new emission shape. Existing fixtures (small/medium/large) have zero unparsed → snapshots unchanged.Test plan
bun test— 335/335 pass (+23 new across cluster, parser, render, dispatch tests)bun x biome ci .— cleanbun run typecheck— cleanRisk / rollback
The footer emission is gated on non-empty unparsed; today's fixtures have zero, so existing rendering is byte-identical for current data. The
tsc/4bump means archivedtsc/3evidence carries pre-quarantine attribution. Rollback: revert both commits, no migrations.