fix(memory): honor status in add_finding; open friction-log spec#1208
Merged
Conversation
add_finding() never read finding["status"], so every node created through the real public API silently got status="open" regardless of what the caller passed - found reviewing PR #1207's curated-memory NodeTypes (whose whole premise is that these nodes get active/superseded/stale status). Predates #1207 but falsified its design for every node written via the real API; the new regression test asserted .type but not .status, so it didn't catch it. Also opens docs/specs/memory-nodetype-friction-log/ to track real dogfood usage of the 4 new NodeTypes and log taxonomy/field friction separately from implementation bugs like this one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
query() did `for hit in pipeline.run(...)`, but the installed attune_rag's RagPipeline.run() returns a RagResult dataclass (not iterable), so every call raised TypeError, silently swallowed by a broad except and returned []. This means personal_memory_recall - the MCP tool surface - has returned zero results unconditionally since attune_rag's return type changed. Existing mocked tests couldn't catch this: their fake RagPipeline.run() returned a plain list, the same wrong shape the code assumed, so mock and code agreed with each other while both diverged from the real dependency. Fixed: query() now reads rag_result.citation.hits (CitedSource objects with .template_path/.category/.score/.excerpt), matching the pattern already used correctly in workflows/rag_code_gen.py. Updated the 3 affected mocked tests to return the real RagResult/CitedSource shape instead of a plain list, and added a new regression test that exercises the real (unmocked) attune_rag dependency end to end - verified it fails without the fix (TypeError swallowed -> empty result) and passes with it. Found via docs/specs/memory-recall-eval/ - a measure-before-build benchmark (scripts/memory_recall_eval.py) built to answer "does attune.memory's recall actually work" before investing further in the subsystem. Post-fix: hit@1/hit@3 both 100% on an 18-query ground-truth corpus (was 0%/0%). Full writeup + score-distribution caveats in the spec's decisions.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CodeQL review comment on PR #1208 flagged the dead local.
This was referenced Jul 1, 2026
Merged
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.
Summary
add_finding()never readfinding["status"], so every node created through the real public API silently gotstatus="open"regardless of what was passed. Found via/code-reviewof PR feat(memory): add curated cross-session memory NodeTypes #1207 (curated-memory NodeTypes), whose whole premise is that these nodes getactive/superseded/stalestatus — falsified for every node actually written viaadd_finding. Predates feat(memory): add curated cross-session memory NodeTypes #1207.add_findingnow passesstatus=finding.get("status", "open").test_loads_curated_memory_node_via_real_add_finding) to assertstatus == "active"on reload — it previously only asserted.type, so it couldn't catch this.docs/specs/memory-nodetype-friction-log/to track real dogfood usage of the 4 curatedNodeTypemembers and log taxonomy/field friction going forward, separate from implementation bugs like this one (logged there as a "fix note").Test plan
pytest tests/unit/memory/test_graph.py tests/unit/memory/test_nodes_coverage_boost.py tests/unit/memory/test_graph_extended.py tests/memory/test_graph.py— 212 passedpytest tests/agent_factory/test_agent_factory_memory.py— 15 passed, 4 skipped🤖 Generated with Claude Code