fix(scoring): eight copies of has_word_boundary, seven of them panic on the user's own tech stack - #471
Merged
Merged
Conversation
…on the user's own tech stack #422 fixed 17 byte-slicing sites. It could not fix seven it did not know about: the same word-boundary helper had been written **eight times** across the tree, and exactly one copy — `scoring/utils.rs` — was UTF-8 safe. The other seven advanced their search cursor with search_from = abs + 1; // next iteration: text[search_from..] `abs` is the START of a match, so `abs + 1` is a char boundary only when the needle's first char is one byte. And the advance is reached ONLY when the word-boundary test FAILS — i.e. when the match abuts an alphanumeric char. Both conditions must hold, which is exactly why every ASCII test in the tree passed over this for as long as it existed. ("cafe2" is safe; "eclair2" is not, because the term's first char is two bytes.) **The P0 is `signals.rs`.** Verified chain: `scoring/pipeline_v2.rs` -> `clf.classify(..., &ctx.declared_tech, ...)` -> `has_word_boundary(&title_lower, &t)`. `ctx.declared_tech` is the tech stack the user typed at onboarding, and this runs on every item of every scoring pass. One non-ASCII stack entry plus a title where that term abuts an alphanumeric char aborted the whole pipeline. `signals.rs` also had no empty-term guard (N3/N5/N6 did), so an empty stack entry walked the string one byte at a time and panicked at the first ASCII-letter-followed-by-multibyte position. Fixed sites: signals.rs, scoring/dependencies.rs, package_ambiguity.rs, knowledge_decay.rs, dep_linker.rs, preemption.rs, competing_tech.rs, stacks/scoring.rs. The last two take their terms from const tables today, so they were latent — one non-ASCII entry away from live. **The duplication is the actual defect**, so it is gone. The correct implementation is promoted to `utils/text.rs` — not `scoring/utils.rs`, because six of the eight call sites live outside `scoring/` and should not take a dependency on scoring internals for a plain string primitive, and because `utils/text.rs` already houses `truncate_utf8` whose doc comment states the identical rationale. `scoring::utils` now re-exports it, so the ~30 `super::utils::has_word_boundary_match` call sites are untouched. The predicates genuinely disagree and collapsing them would have been a behaviour change, not a refactor, so the shared API is parameterised: - `has_word_boundary_match` — alphanumeric boundaries (5 sites) - `has_word_boundary_match_with_ext` — plus `.js`/`.ts`/`.rs` as a right boundary, for the package-name matchers ("next.js" IS `next`) - `has_bounded_match(.., is_word_char)` — dep_linker and stacks/scoring treat `-`/`_`/`.`/`@` as name-internal - `match_offsets` + `char_before`/`char_at` — for preemption's asymmetric rule (left must be non-alphanumeric, right need only not be a hyphen) and dependencies' `.js`-suffix logic Boundary tests now run on CHARS, not bytes, everywhere. `as_bytes()[i-1] .is_ascii_alphanumeric()` is false for every UTF-8 continuation byte, so a non-ASCII letter glued to the term ("иgo") read as a word boundary and "go" matched — bug E, previously fixed in one copy only. Also fixes a false documented invariant in the same family (`scoring/dependencies.rs`). `has_adjacent_version_literal` took one `name_len` for every position, commented "every accepted form equals the normalized single-token name, so `name_len` is uniform". It is not: `normalize_package_name` strips a leading `@`, so `@foo` yields forms of length 3 and 4. Every `@foo` hit started its version scan one byte INSIDE the name — and did so with unsnapped arithmetic, which panics when that lands mid-char. `package_name_positions` now returns `(offset, form_len)` pairs, so the arithmetic is correct by construction rather than by a premise the next edit would have trusted. Every fix has a test that fails without it — a non-ASCII term (multi-byte FIRST char) abutting an alphanumeric char, asserting both no-panic and the correct boolean — plus one end-to-end test per live entry point (`SignalClassifier::classify` with a non-ASCII declared stack, `compute_competing_penalty` with a non-ASCII primary stack). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…or it reproduced the bug instead of catching it
`llm_judge::parse_judgments` extracted the JSON array with
if let Some(start) = response.find('[') {
if let Some(end) = response.rfind(']') {
&response[start..=end]
`find` scans forward and `rfind` scans backward, so on a garbled or
truncated response the last `]` can PRECEDE the first `[` ("}] text [").
`&response[start..=end]` with `end + 1 < start` panics on a reversed range
rather than erroring — inside a path whose entire job is to survive
whatever a model returns.
The sibling function has been correct the whole time:
`blind_spots::parse_dep_assessments` guards with `(Some(s), Some(e)) if e >= s`
and its doc comment promises "a parse failure yields an empty Vec ...,
never a panic". That guard is ported here, falling through to the raw
response so serde produces the error.
**The test that should have caught this reproduced it instead.**
`hardening_error_path_tests.rs` did not call `parse_judgments` — it
re-implemented the bracket extraction inline, twice, and asserted on
`serde_json::from_str` of the result. So the suite carried its own copy of
the unguarded expression and could not fail no matter what the real
function did. This is the codebase's audited weakness (gates that cannot
fail) in its purest form.
`parse_judgments` is now `pub(crate)` and all four of those tests call it.
Added: reversed brackets error rather than panic (three shapes), and a
multi-byte char between the brackets survives the slice.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ed a network chunk
All three streaming paths — Anthropic SSE, OpenAI SSE, Ollama NDJSON — did
buffer.push_str(&String::from_utf8_lossy(&bytes));
per network chunk. TCP does not respect character boundaries. When a
multi-byte character lands half in one chunk and half in the next,
`from_utf8_lossy` sees an incomplete sequence in BOTH halves and replaces
both with U+FFFD. The character is destroyed before any parser runs.
Not a panic — silent corruption of user-visible LLM output. And it shows
up only on non-English text and emoji, which is precisely the content
least likely to appear in a test fixture, so nothing caught it.
Decoding now happens at a line boundary, where a well-formed stream always
has whole characters: `StreamLineBuffer` accumulates raw `Vec<u8>` and
yields complete lines. `from_utf8_lossy` is still the decoder for a
completed line, so a genuinely malformed line degrades to U+FFFD rather
than dropping the line. Line-splitting semantics are otherwise unchanged
(callers still `.trim()`; a partial trailing line is still held, not
emitted).
Proving tests: a line containing "héllo 世界 🦀" is split at EVERY byte
offset (most of them mid-character) and must reassemble intact with no
U+FFFD; the same payload fed one byte at a time; and one end-to-end check
through `parse_ollama_ndjson`, the parser a streaming path actually calls.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cess, and hang the UI for 30s doing it
`taste_test_respond` took `item_slot: usize` straight off the IPC boundary
and passed it, unvalidated, to `taste_test::inference::update_with_latency`:
assert!(item_slot < NUM_ITEMS, "item_slot out of range");
let likelihoods = &LIKELIHOOD_MATRIX[item_slot];
`assert!`, not `debug_assert!` — live in release. `NUM_ITEMS` is 15. A
trivially malformed `invoke` took the process down.
`ipc_guard.rs` validated strings, URLs and paths and had **no numeric
validator at all**, so this was not an oversight at one call site; every
`usize` crossing IPC reached its consumer unchecked. Added
`validate_range(field, value, max)` following the module's existing
validator conventions (same `FourDaError::Validation` shape, same
`4da::ipc` warn, same "name the field, never echo the value" rule as
`validate_length`), and wired it into `taste_test_respond` BEFORE the
session lookup.
This is also a live instance of the unsettled-promise class: the command
is `async`, so its panic never resolves the frontend's `invoke()` promise
— the UI hangs until the 30s `withTimeout` fires, showing a timeout rather
than an error.
With the boundary validated, `inference.rs` drops to `debug_assert!` (a
development-time contract check for in-process callers) plus a release
backstop: `LIKELIHOOD_MATRIX.get(item_slot)` with a warn-and-no-op instead
of an index panic. An out-of-range slot now leaves the posterior unchanged
rather than killing the app.
Proving tests: `validate_range` at/above/far-above max and the zero-max
edge; `taste_test_respond` rejects NUM_ITEMS, NUM_ITEMS+1, 9999 and
usize::MAX with no active session (proving validation precedes the session
lookup), while slot 0 clears the range check and fails later on the
response string.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… in the process could tell (INV-003)
`cb(changes)` ran unguarded inside the thread spawned by `start_watcher`.
The chain behind that call is `ace/mod.rs::set_callback` ->
`ace/context.rs::process_file_changes` -> `ExtractorRegistry` -> the PDF /
Office / archive extractors — arbitrary parsing of arbitrary user files. A
panic anywhere in there unwinds straight out of `std::thread::spawn` and
kills the watcher thread.
What made it SILENT, precisely:
- `running: Arc<Mutex<bool>>` was set true at startup and cleared only by
`stop()`. It was a private field with no getter — nothing anywhere read
it. The process had the answer and no way to ask.
- The only watcher health surface, `health_checks::check_watcher`, is a
PROXY: it counts `file_signals` rows in the last hour. After the thread
died it reported Healthy for up to an hour, then Degraded with "No file
signals in last hour" — which is the same thing it says about a
developer who spent the afternoon in meetings. A real failure, reported,
and indistinguishable from normal. That is the INV-003 violation, not
the panic itself.
Fixes:
- `cb(changes)` is wrapped in `catch_unwind(AssertUnwindSafe(..))`. On
`Err` the thread logs at ERROR with the panic payload and the user-facing
consequence ("file-derived context will go stale until restart"), clears
`running`, and exits deliberately instead of unwinding.
- `FileWatcher::is_running()` — the flag finally has a reader.
- `ACE::watcher_is_running() -> Option<bool>` (None = no watcher on this
engine: headless, tests).
- `check_watcher` takes liveness and checks it FIRST. `Some(false)` is a
decisive Failed with "Watcher thread is not running — file-derived
context is stale. Restart 4DA." `None` falls through to the row-count
probe, so headless/test behaviour is unchanged. Liveness is passed in
rather than fetched inside the check because the caller already holds the
engine read guard and its `conn` lock.
Proving tests: a callback that panics leaves the watcher `is_running() ==
false` (and the callback demonstrably ran); a healthy callback leaves it
running and `stop()` still clears it; and `check_watcher` returns Failed
with a message naming the real fault even when recent `file_signals` rows
would otherwise report Healthy — the exact hour-long blind spot.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rrect the count that was never re-measured
`Cargo.toml` denies `unwrap_used` crate-wide, which is why production
unwraps sit at exactly zero. It does not configure `string_slice`, which
is the same class of guarantee for the panic vector this branch has been
fixing — and the adoption plan in that comment had exactly one module
carrying `#![deny]` (`source_fetching`).
Twelve more are closed here, plus `utils/text.rs` which is the new home of
the shared word-boundary helper. Each is either at zero hits or has every
remaining slice annotated with `#[allow]` and a stated char-boundary proof:
utils/text.rs, signals.rs, package_ambiguity.rs, knowledge_decay.rs,
competing_tech.rs, dep_linker.rs, stacks/scoring.rs, llm_judge.rs,
llm_stream.rs, ipc_guard.rs, taste_test_commands.rs, health_checks.rs,
ace/watcher.rs
The 11 annotated sites are all provably safe and now say so at the point
of use: `floor_char_boundary` returns a boundary by definition; a byte
offset from `find`/`rfind` of an ASCII needle is a boundary, and so is
that offset plus the needle's ASCII length. `ace/watcher.rs` is the
instructive one — its `find(" from ") + 6` carries a comment about the
`+ 7` bug that used to panic the watcher on curly quotes in copy-pasted
code, and the `#[allow]` now records why `+ 6` is the correct arithmetic.
The comment claimed the lint "fires 280 times". Measured (`cargo clippy
--lib -- -W clippy::string_slice`, this toolchain, before this branch's
fixes) it was 246; after the fixes and these annotations it is 210. The
number moves as modules are hardened, so the note now says to re-measure
before quoting it and records the current shape (ace/scanner.rs 30,
monitoring_briefing.rs 13, scoring/dependencies.rs 12, ...).
NOT promoted crate-wide. The remaining backlog is real, and blanket-
allowing it would cement whatever live panic is still hiding in it — the
same reasoning the original note gave, which stands.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ise it should have pinned Caught by the revert-and-confirm-red pass on the has_word_boundary work: 20 of the 21 new tests went red when their fix was reverted. `compute_competing_penalty_survives_non_ascii_stack` did not — it passed in both states, so it proved nothing. The reason is structural, and is the same reason this call site was classified latent rather than live: `compute_competing_penalty` only reaches `has_word_boundary` when the user's stack entry is a KEY of the `COMPETING_TECH` const table (otherwise it `continue`s), and the other call passes a competitor from that same table. A non-ASCII user stack entry can never get past the lookup, so an end-to-end panic test there passes whether the helper is fixed or not. Replaced with a test of the actual premise: every key and competitor in `COMPETING_TECH` is ASCII. That is falsifiable — the day someone adds a non-ASCII entry it fails and says so, pointing at the unit test where the real guarantee lives. The unit test (`word_boundary_multibyte_term_does_not_panic`) does go red on revert and is unchanged. A test that cannot fail is worse than no test, because it is counted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g_slice — 17 sites audited, all safe
Completes the module-by-module adoption for every module this branch
touched. `preemption.rs` (5 sites) and `scoring/dependencies.rs` (12) were
left open in the previous pass because they needed a real audit rather
than a rubber stamp.
All 17 are provably safe and now say so at the point of use. Three
distinct proof shapes, and the annotations name which one applies:
- `floor_char_boundary` / `snap_to_char_boundary` — a boundary by
definition (context windows, the 30/80-byte advisory caps, the
version-literal scan window).
- a `find`/`rfind` offset of an ASCII needle, optionally plus that
needle's ASCII length — `extract_advisory_id` folds with
`to_ascii_uppercase` precisely so offsets stay valid in the original;
`"] "` is 2 ASCII bytes so `bracket_end + 2` is the match end.
- a `char_indices`/`match_indices` offset, or the end of an exact byte
match — `find_mentioned_version`'s `nearby[pkg_lower.len()..]` is safe
because `nearby` STARTS at a `match_indices(pkg_lower)` offset, so
those first bytes are that occurrence.
Two sites deserved more than a one-liner. In `classify_term_occurrence`,
`text[..pos.saturating_sub(1)]` and `after_str[1..]` are safe ONLY because
they sit inside a `Some('.')` match arm — the matched char is the 1-byte
'.', which is what makes `pos - 1` and index 1 char boundaries. Hoist
either expression out of its arm and the panic is back. That coupling is
now stated in the code, because it is exactly the kind of implicit
invariant the next refactor breaks silently.
No new bugs found — which is the honest result, and worth recording: the
17 sites were already correct, they simply had no machine-checkable proof
that they would stay correct.
One mechanical trap worth knowing: `#[allow]` on a `format!` invocation is
silently ignored ("the built-in attribute will be ignored, since it's
applied to the macro invocation"), so `truncate` binds the slice to a
`let` first. An attribute that does nothing is a suppression you think you
have and do not.
Measured after this change: the lint fires 193 times tree-wide, down from
246 before this branch.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nder dogfoods Follow-up to the IPC range validation. That commit downgraded `assert!(item_slot < NUM_ITEMS)` to `debug_assert!` and added a `LIKELIHOOD_MATRIX.get()` fallback behind it. The downgrade was half-right; the `debug_assert!` should not have stayed. Two reasons it was wrong: 1. **It panics in debug builds — which is what gets used.** `cargo test`, `pnpm run tauri dev` and the founder's daily dogfood all run with `debug_assertions` on. Calling it "a development-time contract check for in-process callers" reads as harmless and means "crashes the app I use every day", for a condition `ipc_guard::validate_range` already rejects at the boundary. CLAUDE.md is explicit: never panic in production Rust, use graceful fallbacks. 2. **It made the fallback untestable, which the previous commit's own report had to admit.** Any test proving the release behaviour tripped the assert first, so the `else` branch could never be exercised. An untested backstop is a guess wearing a guarantee's clothes — and this branch has spent its whole length removing exactly that. The graceful path is now the only path, and it is tested: an out-of-range slot leaves the posterior, the response history and `shown_slots` all untouched (a no-op, not merely a non-panic), and a valid slot still works immediately afterwards — the bad input cannot poison the session. Reverting the guard to `&LIKELIHOOD_MATRIX[item_slot]` turns that one test red and leaves the other nine green. Also corrects `NUM_ITEMS`'s doc comment, which pointed at the `debug_assert!` that no longer exists. It now names `ipc_guard:: validate_range` as the place an out-of-range slot is REJECTED, and this function as the place it merely degrades — a distinction worth keeping straight, since only one of them is the guarantee. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`utils/text.rs` (703) and `llm_stream.rs` (710) both crossed the 700-line warn threshold on this branch. Warnings, not errors — but the fix is cheap and the right cut differs per file, so neither gets the lazy treatment. **`utils/text.rs` -> `utils/word_boundary.rs` (a concern split).** The module was doing two unrelated jobs: preprocessing content for embedding (strip HTML, decode entities, collapse whitespace, chunk) and locating a token inside text. They were only ever neighbours because both handle strings. The word-boundary half is now the single home of a helper that had been written eight times across the tree, so giving it a named module makes it findable — which is the actual defence against a ninth copy being written. 703 -> 484 + 258, and the `#![deny(clippy::string_slice)]` follows the code into the new module. **`llm_stream.rs` -> `llm_stream_tests.rs` (a test split).** Its production code is ~420 lines and is one coherent concern — three provider loops over a shared line buffer. Splitting the streaming logic to satisfy a line count would be damage, so the fixtures move out instead, via the repo's existing `#[cfg(test)] #[path = "*_tests.rs"] mod tests;` convention (~15 files already do this, and `*_tests.rs` is exempt from the warn threshold by design). 710 -> 475 + 245. Test count went 4415 -> 4416 across both moves: +1 for the new out-of-range inference test and 0 lost, which is the check that matters when relocating ~500 lines of test code. Cargo.toml's `string_slice` note is re-measured (193, from 246 before this branch), lists all 18 closed modules, and now spells out what counts as a char-boundary proof so the next module to be closed has a standard to meet rather than a precedent to imitate. It also records that the denies reach `#[cfg(test)]` code deliberately — a test that byte-slices is a test that can panic on its own fixture, which this branch hit for real: the `@fooé` offset assertion now uses `.get(..)`, so it proves the recorded range is a VALID char range as well as the right one. Also corrects the note's own history: the brief for this work said one module carried the deny; it was two (`source_fetching/mod.rs` and `fetcher.rs`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…th still lied
Two defects in this branch's own INV-003 work, both found in review.
1. A CONSTRUCTED-BUT-UNSTARTED WATCHER REPORTED AS FAILED
`watcher_is_running()` mapped straight to `is_running()`, and `running` starts
false. So a watcher that had never been started was indistinguishable from one
whose thread had died, and `check_watcher` short-circuits `Some(false)` to
`HealthStatus::Failed` with "Restart 4DA."
That is reachable in three perfectly healthy states:
- every cold start, because the first health tick fires immediately
(`last_health_check` starts at 0) while `ace_start_watcher` runs at the tail
of the ACE task, after frontend-ready, a grace sleep, a project scan, git
mining and README indexing;
- all configured watch paths missing (renamed project dir, offline network
drive) — `start_watching` never runs, so the state is PERMANENT;
- no context dirs configured, where the ACE task returns before spawning but
`get_ace_engine()` still lazily constructs the engine.
Before this branch those users saw "Degraded — no file signals in last hour".
Telling them their watcher has failed and to restart the app is worse than the
proxy it replaced. `started` is now a latch set when the thread starts, and
`watcher_is_running()` returns `None` — "liveness is not a meaningful question
here" — until it is set. The health check already handles `None` by falling
through to the row-count probe.
2. THE `Disconnected` EXIT LEFT `running == true` ON A DEAD THREAD
`running` was cleared only in the panic arm. The `Disconnected` arm breaks out
of the loop with no panic at all — if the notify backend drops its sender (OS
handle closed, backend thread gone), the thread stops while `is_running()` keeps
returning true. That is the same silently-dead watcher this branch exists to
eliminate, reached through a different door: the fix closed one exit and left
the other open.
Clearing it after the loop covers every exit, deliberate or not.
TESTS
Two, and both are about the distinction rather than the mechanism, because the
mechanism was never the hard part:
- a fresh watcher reports `has_started() == false`, which is what lets a
caller tell "never started" from "died";
- `stop()` clears `running` but LEAVES `started` set, so a deliberately
stopped watcher still reports `Some(false)` rather than reverting to "not a
meaningful question".
Neither constructs a real OS watcher. The existing panic test does, and that is
appropriate for what it proves; making liveness depend on inotify capacity would
put a CI box with exhausted watches into the same false-failure class this
commit is removing.
4412 tests pass. `cargo fmt --check`, `cargo clippy --lib -- -D warnings` and
the file-size gate are all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
runyourempire
force-pushed
the
worktree-agent-a01f9610ade32c536
branch
from
August 16, 2026 17:30
23b0c01 to
e232dc1
Compare
Collaborator
Author
Pre-merge review — MERGE WITH FOLLOW-UP, two mediums now fixedAn adversarial review verified the boundary work empirically rather than trusting the tests, and it holds up well:
Two mediums were worth fixing before merge, both in this branch's own INV-003 work. Fixed in A never-started watcher read as a dead one. One exit path still lied. Deliberately not fixed here, from the same review: Also noted: |
runyourempire
added a commit
that referenced
this pull request
Aug 17, 2026
…commands.rs, stale affinity docs (#479) ## The residual this closes The half of #474 that was skipped because `src-tauri/src/commands.rs` was claimed by the #471 lane (now merged). Inventory item 3 of the v20a dead-surface sweep, tracked in `.claude/plans/PENDING-DECISION.md` item 2. ## Removed from `commands.rs` (all structurally dead, same proof classes as #474) - **`mcp_score_autopsy` "Learned Affinity" component** — `bd.affinity_mult` is pinned `1.0_f32` (`pipeline_v2.rs:614`), so `(bd.affinity_mult - 1.0).abs() > 0.01` could never fire. Pinned-input class. - **`mcp_score_autopsy` "Anti-Topic Penalty" component** — `bd.anti_penalty` is pinned `0.0_f32` (`pipeline_v2.rs:615`), so `> 0.01` could never fire. Pinned-input class. - **`matching_affinities` build + `"learned_affinities"` emit** — reads `ace_ctx.topic_affinities`, which the AD-029 quarantined loader returns empty; always `[]`. Empty-map-read class. The FE render side was already removed in #474; the autopsy type has no `learned_affinities` field. - **`score_tuning_snapshot`'s `feedback_interaction_count`** — mislabeled: its value was `ace_ctx.topic_affinities.len()` (always 0 via the quarantined loader), not a feedback-interaction count. The real `feedback_interaction_count` concept (scoring-context bootstrap detection, taste-test seeding) is untouched. `commands.rs` now has zero `topic_affinities` references. The capture side (`run_background_behavior_decay` → `apply_behavior_decay`) is deliberately untouched — that is v20b operator territory. ## Also cleaned - `src/components/ScoreAutopsy.test.tsx` — stale `learned_affinities` fixture line (the interface field was removed in #474; the fixture carried dead data). - **Two docs still described the retired scoring path as live** (discovered while verifying consumers): - `docs/GETTING_STARTED.md` told users scores come from "Topic Affinity: Learned preferences from your interactions" and an affinity/anti-penalty formula. Replaced with the actual scoring inputs (context similarity, interests, stack/deps, freshness/quality). - `docs/ARCHITECTURE-DETAILED.md` — the PASIFA formula's Steps 5–6 (affinity multiplier, anti-topic penalty), the pipeline flow's "Apply affinity multiplier (learned behavior)" step, and the Relevance Judge diagram entry. Replaced with the v20 reality + a pointer to the PASIFA V2 DSL. Capture-side descriptions (tables, interaction writers, decay) are left as-is — they are still true and pending the operator's v20b call. ## NO PIPELINE_VERSION bump Nothing here touches scoring — only presentation surfaces (autopsy JSON, dev-time tuning snapshot) that provably emitted nothing, plus docs. Stored scores cannot differ. ## Verification - `cargo fmt` clean; `cargo test --lib commands::` targeted pass; clippy via pre-push gate - `npx vitest run src/components/ScoreAutopsy.test.tsx` — 5/5 - `npx tsc --noEmit` clean; `npx eslint` clean on touched TS - `node scripts/check-file-sizes.cjs` — no new errors (commands.rs shrank 40 lines) - `grep topic_affinities src-tauri/src/commands.rs` — zero hits 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LBY5q2MskiKMkTm6NmB49L Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
runyourempire
added a commit
that referenced
this pull request
Aug 17, 2026
…word-boundary fixes (#482) #471 merged scoring-visible changes (unified `has_word_boundary` used by signal classification against the user's declared tech; version-literal offset snapping) with **no version bump**, and the v20 drain completed *before* it merged — so the entire 15.9k corpus is stamped with the pre-#471 matcher. Same dark-fix class the v9 bump repaired (documented at the constant). A live specimen of this class was observed 2026-08-17: a Lemmy Spider-Man post classified as a react "version update" in knowledge-gap evidence. No logic change in this commit — the bump makes the drain re-stamp the corpus with the merged logic. Unregistered (full drain), same cost basis as v20 (~7 cycles, minutes). Activation (rebuild BOTH root-tree binaries — currently split: engine at 03:44, fourda.exe at 02:21 — then one-shot drain + probe) follows the merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMarGhXjbKyNJzsm3JG1jw --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Closes the UTF-8 panic vectors that #422's sweep did not know about, plus the INV-003 violation and the unvalidated numeric IPC boundary.
cargo fmt --checkclean ·cargo clippy -- -D warnings(the repo's gate) clean ·cargo test --lib4,415 passed, 0 failed · file-size gate clean.Eight copies of one helper, seven of them broken
has_word_boundary_matchexists in eight places. Exactly one was correct. The other seven advanced the search cursor withsearch_from = abs + 1, whereabsis the start of a match — which is a character boundary only when the needle's first character is one byte.The worst copy,
signals.rs, takes the user's own onboarding tech stack as its needle and runs on every item in every scoring pass, reached frompipeline_v2.rs→clf.classify(..., &ctx.declared_tech, ...). It also had no empty-term guard, which three of its siblings did have, so an empty stack entry walked the string byte by byte.The brief I was working from was wrong about the trigger, and it mattered
I was told a non-ASCII term abutting an alphanumeric character panics. That is too loose: the term's first character must be multi-byte.
"café"is safe —cis one byte, soabs + 1lands cleanly."éclair"is not.My first pass of tests used
café. Every one of them passed against the unfixed code. I caught it before committing and rewrote all of them withéclair/привет/我们/🦀. Flagging it because a suite of tests that cannot fail is precisely the defect this audit was about, and I nearly shipped one into the fix for it.Two of the seven are not copies
preemption::is_compound_prefix_matchandscoring::dependencies::package_name_positionsshare the defective cursor but implement genuinely different boundary rules — the first is asymmetric (left must be non-alphanumeric, right need only not be a hyphen), the second handles.js/.ts/.rssuffixes and sentence periods. They cannot collapse into one boolean helper without changing behaviour, so they take the newmatch_offsets/char_before/char_atprimitives instead, with the divergence documented in place.Where the shared helper went
utils/text.rs, notscoring/utils.rs. Six of the eight call sites live outsidescoring/, and makingpreemption,dep_linker,competing_techandstacksdepend on scoring internals for a plain string primitive is the wrong direction.utils/text.rsalready housestruncate_utf8, whose doc comment states the same rationale.scoring::utilsre-exports it, so roughly 30 existing call sites are untouched.Four more defects in the same family
A truncated model response panicked the judge.
llm_judge.rsdid&response[start..=end]fromfind('[')andrfind(']')with no ordering guard, so a response whose last]precedes its first[panics. Its sibling inblind_spots.rsalready had the guard and a doc comment promising "never a panic". The test that should have caught it re-implemented the unguarded expression inline instead of calling the function, so the suite reproduced the bug rather than detecting it. Both fixed.Streaming destroyed every multi-byte character straddling a network chunk. All three transports (Anthropic SSE, OpenAI SSE, Ollama NDJSON) called
String::from_utf8_lossyper chunk, so a character split across a packet boundary became U+FFFD in both halves — silent corruption of user-visible output, worst on non-English text and emoji. Now decoded at line boundaries.A false invariant in the version-literal check.
has_adjacent_version_literalcomputed an unsnapped offset while trusting a comment claiming all accepted forms share one length. They do not:normalize_package_namestrips a leading@, so@fooyields forms of length 3 and 4.An unvalidated number from the frontend could abort the process.
taste_test_respondpasseditem_slot: usizestraight from IPC intoassert!(item_slot < NUM_ITEMS)—assert!, notdebug_assert!, so live in release.ipc_guard.rsvalidated strings, URLs and paths but had no numeric validator at all. It does now. Because the command isasync, its panic also left the frontend'sinvoke()promise unsettled until the 30-second timeout — so this was a process abort and a UI hang. The assert is downgraded todebug_assert!with a release backstop that returns rather than indexing out of bounds.INV-003: the file watcher could die and nothing could tell
cb(changes)ran unguarded inside the watcher thread, so a panic anywhere in the extractor chain killed it. What made it silent is worse than the panic: therunningflag it would have set is private with no getter and read by nothing, and the only health surface countedfile_signalsrows in the last hour — so a dead thread reported Healthy for an hour and then looked exactly like an idle developer.The callback is now contained,
runningis cleared on panic and logged at error level, andcheck_watcheris driven by liveness rather than row counts.check_all_componentstakeswatcher_aliveas a parameter rather than fetching it internally, because the one production caller already holds the ACE read guard and the connection lock — fetching inside risked re-entrancy.Verification
I reverted every fix in one pass, keeping all the new tests, and ran the suite: 20 tests went red.
One did not, and that is the useful part.
compute_competing_penalty_survives_non_ascii_stackpassed in both states, becausecompute_competing_penaltyonly reaches the helper when the stack entry is a key of theCOMPETING_TECHconst table — a non-ASCII entry can never get there, so my end-to-end test was vacuous. It is replaced (975ecdf6) with a test of the actual premise: every key and competitor in that table is ASCII, which fails the day that stops being true.Lint adoption
#![deny(clippy::string_slice)]now covers 14 modules, up from one. Eleven remaining slices inside them carry#[allow]with a stated boundary proof. The tree-wide count went 246 → 221 after the fixes → 210 after annotations. TheCargo.tomlcomment quoted a stale 280; it is corrected and now says to re-measure before quoting. No crate-wide deny — the remaining backlog (ace/scanner.rs30,monitoring_briefing.rs13,sources/*~45) is real, and blanket-allowing it would cement whatever live panic is hiding in it.Two caveats
cargo clippy --all-targets -- -D warningsis not clean on this tree — 1,004 findings, pre-existing and toolchain-driven, withtests/victauri_dogfood.rsalone accounting for 579. I verified this branch adds none by intersecting the finding list against its own changed files: the six overlapping files carry only pre-existing findings. The two this branch did introduce were removed before commit.utils/text.rs(703) andllm_stream.rs(710) crossed the 700-line warn threshold. No errors and no gate breakage, but they belong on the split-candidates list.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fq96xWyPQjx2bCCzWtsnC9