fix: rebuild_profiles hermeticity + bridge_finder ValueError on real session data (#174)#180
Merged
Merged
Conversation
…les tests (#174) Two-layer fix for #174. Real bug (production data path): discover_all_memories() returns a list[dict] of memory records, but find_bridges fed it straight into dict.update. A non-empty list of 9-key records raised "ValueError: dictionary update sequence element #0 has length 9; 2 is required". Empty homes skipped the branch, so CI stayed green while every real ~/.claude failed. Root cause: type-contract mismatch, not flakiness. Fix: _index_memories() normalizes the scanner list into an id-keyed map (keyed by path/file/name), leaving the brain_index dict shape untouched. Annotations through profile_assembler updated to the honest dict | list[dict] | None. Test isolation (hidden shared state): three test_rebuild_profiles cases called handler({"force": True}) unmocked, reading the developer's live ~/.claude session data. Replaced with a hermetic_claude_home fixture that redirects every ~/.claude path constant at a synthetic tmp_path seeded with real sessions + memory files, so the full scan -> group -> assemble -> bridge pipeline runs against fixtures and consumes the exact list-of-records shape that triggered the ValueError. Regression pinned at both levels: TestFindBridgesScannerListShape encodes the minimized 9-key record; the handler tests assert the real pipeline scans the fixture memories without error. 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
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 #174
The defect, two layers
1. Real bug (production data path).
scanner.discover_all_memories()returns alist[dict]of memory records, butbridge_finder.find_bridgesfed that list straight intodict.update. A non-empty list of 9-key records raises:Empty homes skip the
if memories:branch, so CI (empty$HOME) stayed green while every developer machine with real~/.claudememory files failed. This is a type-contract mismatch reached by legitimate production data — classified as a real bug, not flakiness.2. Test isolation (hidden shared state). Three
test_rebuild_profilescases calledhandler({"force": True})with no mocking, reading the developer's live~/.claudesession history. Results depended on whose machine ran them.Minimized failing input
A single scanner-shaped record (the 9 keys
file, path, project, name, description, type, body, modifiedAt, createdAt) wrapped in a list:Root-cause classification
bridge_findershould handle that shape — it is exactly what the production producer (discover_all_memories) emits. Fixed there, not by rejecting the data.What changed
core/bridge_finder.py: new_index_memories()normalizes the scannerlistinto an id-keyed map (keyed bypath/file/name); thebrain_indexdictshape is passed through unchanged. Signature widened todict | list | None.core/profile_assembler.py: annotations corrected to the honestdict | list[dict] | Nonethrough the call chain.tests_py/core/test_bridge_finder.py:TestFindBridgesScannerListShapepins the minimized 9-key record and the list+brain_index merge.tests_py/handlers/test_rebuild_profiles.py:hermetic_claude_homefixture redirects every~/.claudepath constant at a synthetictmp_pathseeded with real sessions + memory files; the three previously-live-home tests now drive the full pipeline against fixtures and consume the list-of-records shape that triggered the ValueError.Gates (on this machine, live
~/.claudepresent)ruff check+ruff format --check: pass.pytest: 5304 passed, 9 skipped. The 3 previously-failingtest_rebuild_profilescases are green. Two unrelated pre-existing failures (test_cold_startreal-DB SessionStart hook;test_I2_canonical_writerorder-dependent, passes in isolation) reproduce on cleanmainwithout this change and are out of scope for test: test_rebuild_profiles depends on live ~/.claude data — 3 failures reproduce on main #174.🤖 Generated with Claude Code