feat(audit): account for rows dropped by the search window scan - #42
Merged
Conversation
The quicksearch window scan counted the upstream rows it examined and the rows it withheld by verdict, then threw both away after a debug log, so a search that silently dropped many denied bugs was indistinguishable in the audit stream from one that dropped none (issue #29). quicksearch_window now returns a SearchWindow carrying the served bugs plus the scan accounting, filter_bug_list reports the dropped ids rather than a bare count, and bugs_quicksearch records the numbers as guard.scan on the audit record — the dropped ids ride the existing suppressed_ids machinery and its config switch, while the counts are always recorded. A dropping scan upgrades the verdict to served_filtered; the client response stays byte-identical (I3). The schema-goldens header now states the additive-optional exception this change practices (golden updated in place under v1, prior bytes pinned as a PRE constant with an old-line deserialization test), a multi-chunk fixture pins that the accounting accumulates across scan chunks, and the DESIGN.md bullet states the exact presence rule (every served call records scan, zero-limit as {0, 0}; a failed search records none).
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
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.
Closes #29.
What
bugs_quicksearchaudit records now account for what the window scan withheld.Guard::quicksearch_windowreturns aSearchWindow { bugs, scanned, dropped }— the served window plus the scan's accounting — andGuard::filter_bug_listreports the verdict-dropped ids instead of a bare count. The server records the numbers asguard.scan = {scanned, dropped}on the tool-call record and feeds the dropped ids through the existing suppressed-ids machinery, so they obey the existingsuppressed_idsconfig switch with no second knob, while the counts are recorded regardless of it.Pinned semantics (documented on
SearchWindowand in DESIGN.md):scannedcounts every upstream row examined — duplicate and id-less rows included, because each was fetched and looked at.droppedis exactly the ids the verdict withheld, in scan order: overshoot-region denials (past the requested window but inside the quantised chunk) included; id-less rows and deduped repeats excluded (they never reach a verdict).scanis present on every servedbugs_quicksearchcall — zero-limit records{scanned: 0, dropped: 0}, a failed search records no scan — so on a served search,dropped: 0is a statement, not an omission.served_filtered(previouslyserved) — a search that withheld rows is a filtered serve. A clean scan leaves the verdict untouched.scanoptional-and-absent-when-unset, schema stays v1; the pre-Audit records do not account for rows dropped by the search window scan #29 golden bytes are pinned asGOLDEN_TOOL_CALL_PRE_SCANwith an old-line deserialization test, and the schema-goldens discipline comment now states this additive exception explicitly).Acceptance criteria from the issue, each pinned by a test:
quicksearch_scan_accounting_reaches_the_record_never_the_envelope;quicksearch_response_is_byte_identical_whether_or_not_rows_were_dropped(mixed-vs-clean upstream, auditing on);quicksearch_drop_count_survives_the_suppressed_ids_knob.examples/audit.tomldocuments that the scan-dropped ids follow thesuppressed_idsswitch (counts always recorded); DESIGN.md gains the audit-stream bullet and extended testing bars.Adversarial review record
Implemented via fan-out (implementer → security-bypass / correctness-fail-closed / docs-vs-code lenses + mutation verifier → fixer), findings addressed before this PR was opened. 4 findings, all minor, all fixed:
GOLDEN_TOOL_CALLwithout a version bump, while the change did exactly that (legitimately — additive optional field, blessed by DESIGN.md). Fixed: the comment now states the practiced rule — an optional, absent-when-unset field stays within v1, updates the golden, and must pin the prior bytes as aGOLDEN_*_PRE_*constant with an old-line deserialization test; any other byte change still requires a version bump.{0, 0}without touching upstream. Fixed: DESIGN.md now states the exact presence rule.quicksearch_window_accounting_accumulates_across_chunks(250-row corpus, one hidden id in each chunk) pins the per-chunkextend/+=; both reset-mutants re-proved killed in a throwaway worktree.Mutation verification
16 mutants, 16 killed, 0 survivors: ids never collected;
scannedcounting kept instead of examined rows;scannedcounting post-dedupe rows;note_scannever called;note_suppressed(dropped)removed; verdict upgraded on a clean scan; verdict not upgraded on a dropping scan; suppressed-ids config gate inverted;scannever serialized; accounting leaked into the envelope under a fresh key — killed solely by the byte-identity test, proving it load-bearing for I3; drop count zeroed at the call site; first-write-wins innote_scan; cell state never drained into the record; dropped ids replaced with zeros (killed by the exact-id-list assertions); plus the two per-chunk reset mutants from finding 3.Gate
cargo fmt --all --check·cargo clippy --workspace --all-targets -- -D warnings·cargo test --workspace --all-targets --locked(342 passed, 0 failed) ·cargo deny check(advisories/bans/licenses/sources ok) ·typos— all green, re-run independently after the review fixes.