Make echo_chamber reachable (#150), plus two bugs found while verifying#156
Open
justinstimatze wants to merge 5 commits into
Open
Make echo_chamber reachable (#150), plus two bugs found while verifying#156justinstimatze wants to merge 5 commits into
justinstimatze wants to merge 5 commits into
Conversation
The tuning in fiorastudio#129 left the older detector spec tests behind — they still assume the stricter pre-tuning thresholds, so npm test fails on a fresh clone. This updates the fixtures and expectations to the current values. Tests only, no behavior changes.
…ion (fiorastudio#150) Some detectors are specializations of others. `echo_chamber`'s predicate is `load_bearing_vibes`' predicate plus three conjuncts (user speaker, assistant-only supports, no pushback), so every echo case is also a load-bearing case — one situation at two resolutions, not two competing findings. Selection takes candidates[0] within a category, so the general finding won purely on the order detectors run in. The per-anchor cooldown made it self-reinforcing rather than self-correcting: it keys on anchor_claim_id without regard to type, so emitting the general finding also blocked the specific one on that anchor. `echo_chamber` could not surface at all. `SUBSUMES` names the containment; `runAllDetectors` drops a general finding when a more specific one fired on the same anchor. Thresholds are untouched — Also fixes two defects fiorastudio#129 exposed in the same area: - `unchallenged_chain` fired on the unchallenged *suffix* of a challenged chain. The walk started from every node, so once the minimum length dropped 4→3 a suffix below the challenged pair became a candidate in its own right. It now starts only from maximal chains, matching its documented "anchors on the head" intent. - The "below threshold" / "below minimum length" fixtures hardcoded edge counts that fiorastudio#129's retuning turned into *at*-threshold cases, so they went green while testing nothing. They derive from REASONING_CONFIG now. Three fixtures were mislabeled: they seeded a user vibes claim backed by assistants with no pushback — a textbook echo chamber — and read as load-bearing only because of the ordering bug. They now use an assistant-authored anchor so they test what they claim to. Builds on fiorastudio#151, which fixes the same five failures by pinning the current behaviour and explicitly defers the real fix here. The two fixture sets conflict and should not both be merged as-is. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFi4A9eG3CWPjDDMvdmUfy
…ntical bones The CC-compat rescue path reproduces Claude Code's `Bun.hash()` by shelling out to `bun -e`, passing the userId as a trailing argument and reading it back as `process.argv[1]`. Bun does not forward trailing arguments to `-e` scripts: inside the eval, `process.argv` is `['bun', '<cwd>/[eval]']` and nothing more. `process.argv[1]` was therefore a constant path string, identical on every call, and the userId never reached the hash. Measured on master with Bun installed: 300 distinct userIds produced exactly one distinct stat vector. Rarity, species, eye and stats were the same for every rescued companion. Silent because the roll stayed perfectly deterministic — which is what the determinism test checks — and because the FNV-1a fallback path, used on machines without Bun, was always correct. The input now travels through the environment instead of argv, preserving the no-string-interpolation property that argv was chosen for. After: 200/200 distinct, still deterministic per userId. Already-rescued companions keep their stored bones, so no migration is needed, but every existing rescue was a clone of the same roll. The existing test compared two userIds, which reads as an unlucky fixture precisely when the bug is total. Replaced with a 40-id spread assertion. Not covered by any open PR — fiorastudio#152 and fiorastudio#153 fix the other fresh-clone failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFi4A9eG3CWPjDDMvdmUfy
…test suite resolveSessionTrace walks ~/.claude/projects, ~/.claude/sessions and ~/.codex/sessions to map a session id back to its cwd. It needs the first JSONL record (Claude) or the first dozen (Codex), but read every candidate file in full to get them, so cost scaled with total transcript volume instead of with the header actually parsed. On a machine with real history — 646 files, 8.5 GB — that meant reading 8.5 GB to extract a few hundred kilobytes of headers. reasoning/session-trace.test.ts never finished, and because vitest runs files serially it took the rest of the suite down with it; `npm test` appeared to hang with no failing test to point at. Fresh clones and CI runners have no transcript history, which is why this went unnoticed. Reads are now bounded to a 32 KiB head per file, enough for a first JSONL record by a wide margin. Whole-file .json parsing keeps its own 256 KiB ceiling since it is parsed as a single object. A truncated read discards its trailing partial line rather than trying to parse half a record. Resolution is unchanged — the real workspace still resolves to the same cwd and source, in 4ms. The test goes from a >90s hang to 2s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFi4A9eG3CWPjDDMvdmUfy
The repo has no CI — .github/workflows contained only the stats updater — so nothing has been running the test suite on PRs or merges. fiorastudio#129 shipped a red suite in May and it stayed red until a contributor cloned the repo in July and ran npm test by hand (fiorastudio#150, fiorastudio#151, fiorastudio#152, fiorastudio#153). Node 20 (the engines floor) and 24, npm ci / build / test. `npm ci` compiles better-sqlite3 from source when no prebuild matches the runner ABI, which is the same path users hit on install, so a broken native build fails here too. MERGE ORDER: this gates every PR the moment it lands, and the suite is not green yet. fiorastudio#152 fixes the two doctor check-count failures and fiorastudio#153 fixes the two Penguin frame-width/pose failures; those are the only four remaining, and both PRs are open. Merge them first, or this red-gates unrelated work. Note that CI could not have caught the bug fixed in the preceding commit: a fresh runner has no ~/.claude transcript history, so resolveSessionTrace's unbounded scan costs nothing there. Green CI is not evidence the suite is sound on a real machine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFi4A9eG3CWPjDDMvdmUfy
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
2 participants
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.
Fixes #150.
Stacked on #151 — its commit is the base here, so this PR currently shows it too. Once #151 merges that drops out and only the four commits below remain.
The #150 fix
echo_chamber's predicate isload_bearing_vibes' predicate plus three conjuncts (user speaker, assistant-only supports, no pushback), so every echo case is also a load-bearing case. That isn't a tie to break, it's containment. Selection takescandidates[0], so the general finding won on detector order, and the per-anchor cooldown is type-agnostic, so emitting it also blocked the specific one on that anchor.echo_chambercouldn't surface at all.SUBSUMESintypes.tsnames the containment andrunAllDetectorsdrops a general finding when a more specific one fired on the same anchor. #129's thresholds are untouched — they were tuned against real session density and that work stands.#151 pinned the
echo_chamberassertion and flagged the open question. This answers it, so that assertion goes back to expectingecho_chamber. Three fixtures also turned out to be mislabelled: they seeded a user vibes claim backed by assistants with no pushback, which is a textbook echo chamber, and only read as load-bearing because of the ordering bug.Two bugs found while verifying
Neither is related to #150 — I hit both trying to get a green suite to check the fix against.
Every rescued companion rolled identical bones when Bun is installed.
bun -e '<script>' <arg>doesn't forward trailing arguments, soprocess.argv[1]was a constant and the userId never reached the hash. 300 userIds produced one stat vector. Measured: with Bun on PATH master fails, without Bun it passes, which is why CI would never have caught it.resolveSessionTraceread every transcript in full to get its first line. On a machine with real history that's 8.5 GB across 646 files, and because the scan is synchronous it blocks the vitest worker so the timeout can't fire — the suite looked like it hung with no failing test to point at. Reads are now bounded to a 32 KiB head.CI
There isn't any today, which is how #129 shipped a red suite in May and it stayed red until @InfinitePortaldev cloned the repo in July. Merge #152 and #153 first — they fix the only four remaining failures, and this red-gates every PR the moment it lands.
Verification
Ran the suite against each open PR and the combination:
mastermaster+ #151master+ #152master+ #153master+ #154Each PR fixes exactly its own failures and nothing else. The four commits here touch disjoint files and are independent — happy to split them into separate PRs if that's easier to review, I kept them together to avoid adding six PRs to the queue.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WFi4A9eG3CWPjDDMvdmUfy