Skip to content

feat(mental-models): expose refresh operation model id and list total - #3533

Closed
benfrank241 wants to merge 1 commit into
mainfrom
feat/mental-model-api-observability
Closed

feat(mental-models): expose refresh operation model id and list total#3533
benfrank241 wants to merge 1 commit into
mainfrom
feat/mental-model-api-observability

Conversation

@benfrank241

Copy link
Copy Markdown
Member

Two small observability improvements around mental models. Independent of each other; no schema changes.

1. mental_model_id on operation records

refresh_mental_model operations return document_id: null and 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's result_metadata — this surfaces it as a first-class mental_model_id field on OperationResponse and OperationStatusResponse (null for other task types).

2. total on GET /banks/{bank}/mental-models

The list envelope was {"items": [...]} only and silently capped at limit (default 100), so a bank with more models than the page size looked fully-listed. Adds total (matching-row count, ignoring limit/offset) plus limit/offset echoes, backed by a new count_mental_models() engine method that applies the same tag filter as list_mental_models.

Tests

  • A refresh_mental_model operation surfaces the correct mental_model_id; non-refresh ops don't carry it.
  • Listing with a smaller limit still reports the full total, including a tag-filtered total.

🤖 Generated with Claude Code

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 koriyoshi2041 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@nicoloboschi

Copy link
Copy Markdown
Collaborator

superseded by #3548 including same fix for directives

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants