fix(stella-tools): save_state success output carries the saved content's identity - #3299
Open
macanderson wants to merge 1 commit into
Open
fix(stella-tools): save_state success output carries the saved content's identity#3299macanderson wants to merge 1 commit into
macanderson wants to merge 1 commit into
Conversation
…t's identity
save_state's success string was the last constant-shape output on the
12-tool surface: two saves of different same-length content under one
key rendered byte-identical "saved {key} (N bytes)" outputs, and the
engine's stagnation detector (stella-core's loop_detect, rung 3)
deliberately keys on byte-identical consecutive same-tool outputs — so
a legitimate checkpoint loop (a fixed-width counter updated under one
key) could be killed as stagnant mid-solve, the same hazard class
#3176/#3187 fixed for edit_file before the tool purge (#3244) deleted
that surface and its staleness.rs helper.
The success output now appends the saved content's identity:
saved {key} ({N} bytes, sha256/8 {digest})
where the digest is the first 8 hex chars of the content's sha256,
computed via foundry_gate::digest — the crate's existing hashing
primitive — rather than reintroducing a second Sha256-to-hex helper.
Content-derived only, per the detector's settled contract: no timings,
no randomness (#2706), so a loop genuinely re-saving identical bytes
still renders byte-identical output and is still caught.
Witness (two-sided, in registry/tests.rs beside the existing scratch
dispatch tests): different same-length content under one key must
render distinct outputs — fails on the old format string ("saved
checkpoint (9 bytes)" twice), passes with the digest; and identical
content re-saved must render byte-identical output, which holds on
both sides.
docs/tools/save_state.toml captures no observed example and the
ToolSchema is unchanged, so make tool-docs is unaffected (verified).
Closes #3297
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
Reviewer's GuideUpdates the save_state tool's success output to include a content-derived sha256/8 identity to avoid false-positive stagnation detection, and adds tests and documentation to lock in this behavior. Sequence diagram for updated save_state success outputsequenceDiagram
participant SaveState
participant StdFs as std_fs_write
participant FoundryGate as foundry_gate_digest
SaveState->>StdFs: write(path, content)
alt Ok(())
SaveState->>FoundryGate: digest(content.as_bytes)
FoundryGate-->>SaveState: sha256_hex_64
SaveState-->>SaveState: identity = sha256_hex_64[..8]
SaveState-->>SaveState: ToolOutput::Ok { content = format!("saved {key} (N bytes, sha256/8 {identity})") }
else Err(e)
SaveState-->>SaveState: ToolOutput::error(format!("failed to save {key}: {e}"))
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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
1 participant
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 & why
save_state's success output was the last constant-shape output on the 12-tool surface (#3297's sweep of the other 11 explains why each is exempt): two saves of different same-length content under one key rendered byte-identicalsaved {key} ({N} bytes)outputs, and the stagnation detector (crates/stella-core/src/loop_detect.rs, rung 3) deliberately kills consecutive same-tool calls whose outputs are byte-identical — whatever their arguments. A legitimate checkpoint loop (a fixed-width counter or constant-size JSON blob updated under one key) could therefore be killed as stagnant mid-solve — the same hazard class that converted a correctedit_filerun into anagent_errorloss on Terminal-Bench (#3176), was fixed foredit_file/apply_editsby #3187, and re-arose here because the tool purge (#3244) deletededit.rs/write.rs/staleness.rsand closed #3188 as moot without retiring the class.The success output now appends the saved content's identity:
{digest}is the first 8 hex chars of the content's sha256, computed viafoundry_gate::digest— the crate's existing, documented "one hashing primitive" — rather than reintroducing a second Sha256-to-hex helper to replace the deletedstaleness::sha256_8(no new dependency;sha2was already in use). Content-derived only, per the detector's settled output-keying contract: no timings, no randomness (#2706), so a loop genuinely re-saving identical bytes still renders byte-identical output and is still caught. Thesaved {key} ({N} bytesprefix is stable — the identity is appended, never restructured.ToolSchemais unchanged.Closes #3297
Refs #3176, #3187, #3188, #3244, #2706.
The witness
Stella's definition of done is a test that fails on the old code and passes on the new.
main, passes here)Two tests in
crates/stella-tools/src/registry/tests.rs, beside the existing scratch dispatch tests, checked the two-sided way (format string temporarily reverted to the old constant shape, witness watched failing, fix restored):save_state_outputs_differ_for_different_same_length_content— fails on the old format string, where both saves render byte-identical output:counter=1:saved checkpoint (9 bytes)counter=2:saved checkpoint (9 bytes)(identical — the assertion output wasleft: "saved checkpoint (9 bytes)" / right: "saved checkpoint (9 bytes)")counter=1:saved checkpoint (9 bytes, sha256/8 055ea690)counter=2:saved checkpoint (9 bytes, sha256/8 1424b099)save_state_output_is_identical_for_identical_content— re-saving identical bytes still renders byte-identical output (passes on both sides, guarding the detector's catch of genuinely stuck loops).The gate
cargo fmt --checkcargo clippy -p stella-tools --all-targets -- -D warnings(no other crate pins the message — sweptrg '"saved |saved \{'workspace-wide)cargo test -p stella-tools(222 + 1 integration, all green)make doc-warnings CARGO_SCOPE="-p stella-tools"(rustdoc-D warnings, private items included)make tool-docs— OK;docs/tools/save_state.tomlcaptures no observed example and the schema is unchanged, so no regeneration neededscratch.rsnow states the output-identity contract and why it is content-derivedCloses #3297appears both above and as a commit trailerNothing left behind
e352cf486found no further instance of the class, and I re-checkedrg 'content: format!' crates/stella-tools/src— the remaining constant-shape success string isdelete_state'sdeleted {key}, which carries the repeat-delete exemption the issue documents: a second delete of one key errors, so distinct effects cannot repeat silently.)Ground-rule check
stella-core; no new deps (reusesfoundry_gate::digest, already backed by the existingsha2dependency)Anything reviewers should know?
Alternative rejected: reintroducing a
staleness.rs-stylesha256_8helper (the issue's suggested reference shape).foundry_gate::digestalready exists in this crate and documents itself as "the workspace's one hashing primitive", so a second Sha256-to-hex function would be duplication; the 8-char truncation happens at the one call site, on a value that is always 64 ASCII hex chars.Summary by Sourcery
Ensure save_state success outputs embed content identity to avoid false stagnation detection while keeping identical-content saves byte-identical.
Bug Fixes:
Enhancements:
Tests: