fix: stop evicting in-flight curation decisions on an unrelated publish - #306
Merged
Conversation
Revalidation currently compares the whole comparison manifest, which includes
authorized_corpus_count, a project-wide COUNT of authorized memories. Any
publish anywhere in the project bumps it and evicts every curation decision in
flight, after the judge call has already been paid for.
Red before the fix:
- test_revalidation_ignores_unrelated_authorized_memory_added_after_freeze
(predicate returns False although entries and comparison_complete are
byte-identical)
- test_empty_shortlist_publish_commits_when_unrelated_memory_appears
(settle raises stale_decision: authorized shortlist changed before
settlement)
The remaining new tests are regression guards for the fences that must survive
the change: entry transition advance, in-place body rewrite, a new memory
displacing an entry, comparison_complete regressing, and the full manifest
parity check in _validate_verdict.
revalidate_curation_shortlist compared the whole comparison manifest, and that manifest hashes authorized_corpus_count - a COUNT of every authorized memory in the project. So any publish anywhere in the project evicted every curation decision in flight, after its judge call was already paid for. On the stand that was 45 stale_decision against 46 successful derivations in one 20-minute window. The count is not an input to any decision. build_derivation_facts, feasible_outcomes, derive_decision and _apply_evidence_policy read the candidate tier, comparison_complete and the per-entry facts, and nothing reads the count. It only travels as an informational field in the judge prompt envelope and as an unread field on CurationJudgeResult. Revalidation now compares a separate hash over the entries and comparison_complete. Why entries-only is sound: the shortlist is top-K (MAX_ENTRIES=12), so a new memory that would have ranked into it changes the entries and is still caught; a memory ranking below K could not have entered the comparison set the model judged, so it cannot change the relation set. The count check was never the mechanism that prevented a duplicate either - it aborts on any change without distinguishing what changed, and a retry that rebuilds the shortlist reaches the same verdict, so it delayed rather than prevented. Two remaining, deliberate limits: this is exactly as strong as the shortlist's own recall (a true duplicate the legs never retrieve was never in the manifest to begin with), and a freshly published memory still flips comparison_complete to False until its embedding lands, which keeps evicting in-flight decisions that were frozen with comparison_complete=True. _manifest_hash is untouched, byte for byte: it is stored on CurationDecision.comparison_manifest_hash, carried on CurationJudgeResult, and _validate_verdict still compares it in full, so a verdict judged against another shortlist is still stale_decision. The per-entry payload is now shared by both hashes, and canonical_json_bytes sorts keys, so the manifest bytes cannot drift. Every other fence is unchanged: _shortlist_memory_fence, _candidate_fence, _acquire_targetless_publication_lock, and CurationShortlistError still returning False. The settle-path stub follows production instead of the count: it now asks whether a newly authorized memory would rank into this shortlist, modelled by a title match, which keeps the concurrent targetless publication race honest - two identical candidates still serialize and the loser is still relisted.
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Bundle ReportBundle size has no change ✅ Affected Assets, Files, and Routes:view changes for bundle: engram-frontend-client-array-pushAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #306 +/- ##
=======================================
Coverage 91.09% 91.10%
=======================================
Files 223 223
Lines 23344 23350 +6
=======================================
+ Hits 21266 21273 +7
+ Misses 2078 2077 -1 ☔ View full report in Codecov by Harness. |
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.
What
Shortlist revalidation compares the shortlist the verdict actually depended on — its entries
and
comparison_complete— instead of a hash that also includes a project-wide memory count._manifest_hashand everything it hashes are unchanged.Why
Measured on the stand: in one 20-minute window, 46 successful curation derivations against 45
stale_decision. Roughly half of all judged candidates were thrown away after the providercall was paid for. At the observed rate that was the single largest remaining source of
discarded paid inference — ~$1.8/hour with a 74% discard rate.
_manifest_hash(curation_shortlist.py:181) hashed three things:_revalidate_shortlistrebuilt the shortlist inside the settle transaction and required thewhole hash to match. So any newly published memory anywhere in the project bumped the count
and evicted every curation decision then in flight — including decisions about candidates the
new memory has nothing to do with and would never have appeared alongside. With concurrent
workers on one project, a single publish wasted the calls racing beside it.
authorized_corpus_countis not an input to any decision. Every rung ofderive_decisionreadscandidate tier, per-target facts and
comparison_complete;_apply_evidence_policyreads thesame. The count exists for telemetry and for the stored manifest identity.
Soundness
The shortlist is top-K (
MAX_ENTRIES = 12). A new memory that would have ranked into it changesentriesand is still caught. A memory that ranks below the cut could not have appeared in theshortlist the judge saw, so it cannot change the relation set that was judged. Dropping the bare
count therefore removes evictions that could not have affected the verdict, and only those.
Fences deliberately left intact
_manifest_hashitself — it is stored onCurationDecision.comparison_manifest_hash, carriedon
CurationJudgeResult, and compared in_validate_verdict. Changing it would invalidatestored records and that parity check. The new
shortlist_revalidation_hashis separate._shortlist_memory_fence— target advanced still meansstale_decision._candidate_fence,_acquire_targetless_publication_lock, and the_validate_verdictmanifest parity check are untouched.
revalidate_curation_shortliststill returns False onCurationShortlistError.Tests
comparison_completeregressesstale_decisionwhen the appearing memory would have beenshortlisted
stale_decisionVerified:
pytest -m "not transactional"→ 3490 passed;pytest -m transactional→ 89 passed;engram_curator_eval→ all thresholds pass;ruff checkandruff format --checkclean.Operational note
Curation is currently paused on the stand via
ENGRAM_CANDIDATE_DECISION_ENABLED=0(B-012),because it was burning ~$43/day at a 74% discard rate. The flag lives only in the server's
.env, which a release overwrites from the local copy, so deploying this change lifts the pauseautomatically and the blocked works clear on the configuration-fingerprint change.