feat(mental-models): expose refresh operation model id and list total - #3533
Closed
benfrank241 wants to merge 1 commit into
Closed
feat(mental-models): expose refresh operation model id and list total#3533benfrank241 wants to merge 1 commit into
benfrank241 wants to merge 1 commit into
Conversation
Two observability gaps around mental models made it hard to reason about
refresh activity and pagination:
1. refresh_mental_model operations returned document_id: null and carried no
model identifier in the API response, so a client could not tell which
mental model an operation refreshed. The id already lives in the operation
row's result_metadata; surface it as a first-class `mental_model_id` field
on OperationResponse and OperationStatusResponse (null for other task types).
No schema change.
2. GET /banks/{bank}/mental-models returned {items: [...]} only and silently
capped at limit (default 100), so a bank with more models than the page size
was indistinguishable from a fully-listed one. Add `total` (matching-row
count, ignoring limit/offset) plus `limit`/`offset` echoes to
MentalModelListResponse, backed by a new count_mental_models() engine method
that applies the same tag filter as list_mental_models.
Tests: refresh op surfaces the correct mental_model_id (and non-refresh ops
don't); list returns the full total under a smaller limit, including a
tag-filtered total.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
koriyoshi2041
suggested changes
Aug 17, 2026
koriyoshi2041
left a comment
Contributor
There was a problem hiding this comment.
The API shape and focused tests look consistent, but this head cannot merge yet because the checked-in generated surface is stale. verify-generated-files regenerated 10 files (347 insertions / 9 deletions), including the Go/Python/TypeScript response models plus both OpenAPI copies. Please run the generation commands printed by the job and commit those outputs.
The separate API shard failure looks unrelated to this diff: 1,940 tests passed and the only failure was a PostgreSQL deadlock in test_import_bank_creates_per_bank_indexes. I would treat that as a rerun candidate after the generated files are updated.
Collaborator
|
superseded by #3548 including same fix for directives |
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.
Two small observability improvements around mental models. Independent of each other; no schema changes.
1.
mental_model_idon operation recordsrefresh_mental_modeloperations returndocument_id: nulland no model identifier, so a client can't tell which mental model an operation refreshed (diagnosing refresh behaviour from the operations log is impossible). The id already exists in the operation row'sresult_metadata— this surfaces it as a first-classmental_model_idfield onOperationResponseandOperationStatusResponse(null for other task types).2.
totalonGET /banks/{bank}/mental-modelsThe list envelope was
{"items": [...]}only and silently capped atlimit(default 100), so a bank with more models than the page size looked fully-listed. Addstotal(matching-row count, ignoring limit/offset) pluslimit/offsetechoes, backed by a newcount_mental_models()engine method that applies the same tag filter aslist_mental_models.Tests
refresh_mental_modeloperation surfaces the correctmental_model_id; non-refresh ops don't carry it.limitstill reports the fulltotal, including a tag-filtered total.🤖 Generated with Claude Code