fix(cli): parse get-memory response correctly - #2111
Merged
Conversation
The `hindsight memory get` command deserialized the API response into a
local `MemoryUnitDetail` struct whose shape had drifted from what
`GET /memories/{memory_id}` (MemoryEngine.get_memory_unit) actually
returns:
- `entities` is a flat list of canonical-name strings, but the struct
expected a list of `{id, name}` objects, so serde failed whenever a
memory had entities — surfaced to users as the misleading
"Invalid API response format".
- the fact type is exposed as `type`, but the struct renamed it to
`fact_type`, so the Type line always printed UNKNOWN.
The endpoint returns an untyped JSON body in the OpenAPI spec, so the
generated client never validates it and the mismatch only blew up in the
CLI handler. These commands had no test coverage (docs use curl).
Fix the struct to match the response and add regression tests.
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.
Problem
hindsight memory get <bank> <id>fails with a misleading "Invalid API response format" error whenever the memory has any entities.The CLI deserializes the response into a hand-written local
MemoryUnitDetailstruct that had drifted out of sync with whatGET /memories/{memory_id}(MemoryEngine.get_memory_unit) actually returns:entities— the API returns a flat list of canonical-name strings (["Alice", "Bob"]), but the struct expected a list of{id, name}objects.serde_json::from_valueerrors on the type mismatch, and the friendly error layer renders it as "Invalid API response format".type, but the struct renamed it tofact_type, so even when parsing succeeded theType:line always printedUNKNOWN.The endpoint returns an untyped JSON body in the OpenAPI spec, so the generated client never validates it — the mismatch only blows up in the CLI handler. These commands had no test coverage (the docs only demonstrate them with
curl), so the drift went unnoticed.Fix
entities: Option<Vec<String>>(wasVec<EntityRef>); removed the now-unusedEntityRefstruct.fact_type→type.get_memory_unit.Tests
Added two regression tests deserializing realistic API payloads (full + minimal). Both fail before the fix, pass after.
cargo testgreen;./scripts/hooks/lint.shclean.Notes
hindsight memory historywas mentioned alongside this in a bug report but is not affected — it pretty-prints the raw JSON value with no struct parsing. A missing/non-observation memory there returns 404 ("API request failed (404)"), not a format error.