Repository navigation
fix(structured): reserve all four provenance fields, not two - #522
Merged
Merged
Conversation
The structured path copied a candidate's metadata wholesale and then
overwrote only `compiler` and `compiler_version`. A producer-supplied `model`
or `prompt_version` therefore survived into the compiled memory, and
GET /admin/subjects/{s}/memories/{id}/compiler-trace reads `model` straight
out of that metadata, so it would report a model that never touched the
memory. That contradicts the stated intent that these fields are set by
Statewave, and the test named for it only checked the two keys that were
already safe.
Filters all four reserved keys before stamping, and extends the test to
supply `model` and `prompt_version` as well. Verified by mutation: reverting
the filter fails test_structured_compiler_provenance_is_reserved, restoring it
passes.
Also corrects the heuristic comment, which still said metadata_ "stays {}
exactly as before" when it now always carries the compiler provenance, and
adds the two missing trailing newlines.
ruff clean, 1206 unit tests and 321 integration tests pass.
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.
Follow-up to #521, which I merged a moment ago.
The gap
#521 stamps compiler provenance into every compiled memory, which is what #443 asked for. The structured path, though, copies a candidate's metadata wholesale and then overwrites only two of the four provenance keys:
So a producer-supplied
modelorprompt_versionsurvives into the compiled memory. That matters becauseserver/api/admin.py:1383readsmodelstraight out of this metadata, soGET /admin/subjects/{s}/memories/{id}/compiler-tracewould report a model that never touched the memory.It also contradicts #521's own stated intent that these fields are set by Statewave, and the test named for it (
test_structured_compiler_provenance_is_reserved) only supplied the two keys that were already safe.The fix
Filters all four reserved keys before stamping, and extends that test to supply
modelandprompt_versiontoo.Verified by mutation rather than by reading: reverting the filter makes
test_structured_compiler_provenance_is_reservedfail, restoring it passes.Also corrects a comment in the heuristic compiler that still said
metadata_"stays{}exactly as before" when it now always carries the provenance, and adds two missing trailing newlines.Checks
ruffclean, 1206 unit tests and 321 integration tests pass locally.