Skip to content

fix(core): hash every list element and compare messages by value - #3842

Merged
jujn merged 3 commits into
agentscope-ai:mainfrom
ningmao-hlyz:fix/state-list-content-hash
Oct 29, 2026
Merged

jujn merged 3 commits into
agentscope-ai:mainfrom
ningmao-hlyz:fix/state-list-content-hash

Conversation

@ningmao-hlyz

@ningmao-hlyz ningmao-hlyz commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

AgentScope-Java Version

2.1.0-SNAPSHOT (main at c4bab46)

Description

Fixes #3841
Fixes #1357

ListHashUtil decides whether a state list needs a full rewrite by hashing it and comparing the result with the hash stored on the previous save. Both sides of that comparison are wrong today, and they are wrong in opposite directions.

#1357 was filed against 1.0.13, but neither half has been addressed on main: ListHashUtil was last touched in June by #1670, which only moved the package, and no message class has gained equals / hashCode since. The lost-write case below reproduces on 2.1.0-SNAPSHOT (c4bab46) with the plain JsonFileAgentStateStore.

The hash only reads part of the list

computeHash sampled five positions (0, size / 4, size / 2, size * 3 / 4, size - 1) once a list grew past five elements. A sample is a fine fingerprint when all you need is to spot change; it is not fine for the question actually asked here, which is "is the prefix I persisted still the same?". An element replaced at any other position leaves the hash untouched, needsFullRewrite sees a match, values.size() > existingCount is false because the list did not grow, and the save takes neither the rewrite branch nor the append branch — it returns successfully having written nothing.

hashBefore = e0933361
hashAfter  = e0933361
needsFullRewrite = false
reloaded index 3 = "m3"        (expected "EDITED")

computeHash now reads every element. A save already serializes the whole list to JSON and commits it to storage; one hashCode() per element is noise next to that.

Where this actually bites

InMemoryMemory.saveTo and StateBackedMemory.saveTo hand the full List<Msg> to the store on every save, and the InMemoryMemory Javadoc states the contract: the store "appends only new items based on file line count". The whole incremental scheme rests on needsFullRewrite being right.

ToolResultEvictionMiddleware.prepare is the easiest way to reach the failing shape — it replaces evicted tool results in place (contextMutable().clear() then addAll(candidate.history())), so the list keeps its length while entries in the middle change. If the evicted slot is not one of the five sampled positions, the eviction is never persisted and the agent re-sends the full result on every later turn.

Equal messages hash differently

Msg, ChatUsage and every ContentBlock subclass implement State but keep identity equals / hashCode. Two lists built from equal messages therefore hash differently, so every save rewrites the whole list — the behaviour reported in #1357 — and a list rebuilt from storage always looks modified.

This change gives them value semantics, following the pattern AgentState already uses:

@Override
public boolean equals(Object o) {
    if (this == o) {
        return true;
    }
    if (!(o instanceof TextBlock other)) {
        return false;
    }
    return Objects.equals(text, other.text);
}

Why a helper instead of plain Objects.equals

Msg and ToolUseBlock keep provider metadata in a Map<String, Object>, and some of those values are arrays — Gemini stores its thoughtSignature as a byte[]. Map.equals compares values with Object.equals, and arrays compare by identity, so a deserialized message would still look different from the message it came from. ValueEquality (package-private, core.message) recurses into arrays, maps and lists, so both equality and hashing are decided by content.

That is the difference from #1364, which took the other route: it normalized the signature to a Base64 string, which meant touching GeminiMessageConverter, GeminiResponseParser and OpenAIReasoningDetail as well, and changing the serialized form of the metadata with them. Keeping byte[] as byte[] leaves the wire format alone and keeps this change inside core.

Msg.equals also covers usage. It is persisted (@JsonProperty) but was missing from #1364's comparison, and a usage change that does not move the hash is exactly the class of bug being fixed here.

Behaviour change worth flagging

CompactionMiddleware.sameSerializedHistory uses canonical.equals(conversation) as a fast path to skip writing back a conversation that did not change. With identity equality that comparison could only be true for the same objects; with value equality it is true for equal content, which is what the method name says it wants.

The visible consequence: a conversation whose metadata cannot be serialized is now written back when it was not edited, where before the equality check failed first and the write was skipped. ContextPipelineV2Test.unserializableMetadataSkipsWritebackInsteadOfFailing is split in two to cover both paths — an edited view still skips the writeback through the JsonException branch, an unedited view now commits.

The other List<Msg> equality checks in core and harness all ask "did the content change", so this is the behaviour they were written for.

Tests

  • ListHashUtilTest: the hash moves when only a middle element changes; needsFullRewrite detects an edit at a position the old sample never read.
  • JsonFileAgentStateStoreTest: end-to-end regression — save, replace index 3, save, read back. Plus the pair that pins both sides of the identity question: a rebuilt-but-equal list leaves the file alone (mtime unchanged), a rebuilt-and-different list triggers the rewrite.
  • MsgTest: separately built messages compare equal, binary metadata compares by content, usage participates.
  • ContentBlockEqualityTest: every ContentBlock subtype survives a JSON round trip with an equal hash, and a changed field is visible.

Verification

  • mvn -pl agentscope-core test: 2,880 passed, 0 failures, 8 skipped
  • mvn -pl agentscope-harness -am test: 1,462 passed, 0 failures, 33 skipped
  • mvn spotless:check: clean

Reverse check: swapping Msg.hashCode for identityHashCode makes three of the new tests fail, and saveListSkipsRewriteForRebuiltEqualMessages fails by observing the file being rewritten (the mtime moves), which confirms the tests measure the behaviour rather than the API.

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has been formatted with mvn spotless:apply
  • All tests are passing (mvn test)
  • Javadoc comments are complete and follow project conventions
  • Related documentation has been updated (e.g. links, examples, etc.)
  • Code is ready for review

…lost

computeHash sampled five positions once a list grew past five elements.
needsFullRewrite uses that hash to decide whether the prefix already
persisted still matches, and an edit at any other position left the hash
untouched: the store took neither the rewrite nor the append branch and
returned successfully having written nothing.

Read every element instead. A save already serializes the whole list to
JSON and commits it to storage, so one hashCode() per element is noise
next to the write it guards.
Msg, ChatUsage and every ContentBlock subtype implement State but kept
identity equals and hashCode, so two lists built from equal messages hashed
differently and every save rewrote the whole list.

Add value semantics following the pattern AgentState already uses. Metadata
is compared through ValueEquality, which recurses into arrays, maps and
lists: provider metadata carries byte[] payloads (Gemini's thoughtSignature)
and array equality is identity based, so a deserialized message would
otherwise still look different from the message it came from. Msg.equals
also covers usage, which is persisted but was missing from the earlier
attempt in agentscope-ai#1364.

Document the contract on State: implementations stored in lists must compare
by content.

The value semantics also make CompactionMiddleware's sameSerializedHistory
fast path behave as its name says. A conversation whose metadata cannot be
serialized is now written back when it was not edited, where the failing
equality check used to skip the write first; the corresponding test is split
in two to cover both paths.
@ningmao-hlyz
ningmao-hlyz force-pushed the fix/state-list-content-hash branch 2 times, most recently from 2c7a123 to c9a6e2a Compare October 29, 2026 06:42
@jujn
jujn force-pushed the fix/state-list-content-hash branch from c9a6e2a to cff4993 Compare October 29, 2026 07:01
@jujn
jujn merged commit b067261 into agentscope-ai:main Oct 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants