fix(documents): answer a memories-shaped write body with the document shape (09/17 m-13) - #1688
Conversation
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
|
@Eldad-Caura please approve |
242904a to
edf8ebb
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
edf8ebb to
71861a7
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
71861a7 to
a74600a
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
a74600a to
717eb46
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
717eb46 to
34bc677
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
34bc677 to
934fdca
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
934fdca to
3c476ec
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
… shape (09/17 m-13)
Documents store their payload under `data`; memories store it under
`content`. Nothing on the surface said so, so an agent that had already
used `POST /memories` sent `{"title": ..., "content": ...}` to
`POST /documents` and got the generic unknown-field 422 plus "Field
required" for `collection`, `doc_id` and `data` — three fields it had
never been told about. The response named everything wrong with the
request and nothing about what a right one looks like.
The message is the fix, because the naive body cannot be made to
succeed. The two fields it is missing are the two that cannot be
invented:
* `doc_id` — the write is an upsert idempotent on
`(collection, doc_id)`. Minting an id server-side would silently
convert it into create-every-time: the same call twice leaves two
rows, a retry after a timeout duplicates instead of converging, and
the C34 shrink guard would have no stored row to compare against.
That is a change to what the endpoint means, sold as a convenience.
* `collection` — a default is a shared namespace, and the doc_id
inside it is the upsert key, so two callers that both accept the
default and pick the same obvious doc_id overwrite each other.
So the request fails either way and the only open question is what it
is told. A `mode="before"` validator on `DocWriteRequest` now fires when
the body borrows a payload name from the memories surface (`content`,
`text`, `body`, `title`, `summary`, `memory_type`, `metadata`) and
answers with the document shape: where the payload goes, what the
`(collection, doc_id)` pair means, which fields this body is missing,
a minimal body that can be sent as-is, the consequence of omitting
`data["summary"]`, and — when `content` is present — a pointer at
`POST /memories` in case that was the store the caller wanted. Same
move as the `NO_SUCH_ROUTE` 404 from #1636, which answers a wrong path
by naming the real ones.
Deliberately NOT an alias into `data`. Accepting a top-level `content`
would not rescue the body that motivated this (collection and doc_id
are still required), and would buy a second spelling for the payload —
plus a precedence rule for bodies that send both — on a store whose own
history says extra spellings of a body field cannot be made correct
(CAURA-717).
The list is named, not a category, matching `SERVER_OWNED_MEMORY_FIELDS`:
any other unrecognised key is still just a typo and keeps the ordinary
SAFE-01 unknown-field 422.
Also describes `collection` / `doc_id` / `data` in the published schema,
where `data` was an undescribed `object` — a caller reading the contract
had to guess too. No behaviour change and no broker-contract change
(`openapi.broker.json` regenerates byte-identical; documents are not
broker operations).
Every request that changes behaviour here was already a 422 and still
is, so no existing caller is affected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Arkady Mankovsky <arkash20@gmail.com>
3c476ec to
ac1fcca
Compare
|
Claude Code Review — skipped: PR author 'arkash20' is not a public member of the 'caura-ai' org |
🤖 I have created a release *beep* *boop* --- <details><summary>backend: 3.18.1</summary> ## [3.18.1](backend-v3.18.0...backend-v3.18.1) (2026-09-23) ### Bug Fixes * **contradiction:** stop Path C orphaning a conflicted row (09/22 L-12) ([#1690](#1690)) ([469f597](469f597)) * **documents:** answer a memories-shaped write body with the document shape (09/17 m-13) ([#1688](#1688)) ([9f3aecc](9f3aecc)) * **forge:** stop skipped clusters from spending the run's write budget ([#1687](#1687)) ([5132e20](5132e20)) * **memory:** make caller ownership of summary/tags durable on the row (09/02 L-08) ([#1693](#1693)) ([6cce76a](6cce76a)) * **sentinel:** scan name + tags, and settle the support_files checks (09/02 L-01, L-02) ([#1689](#1689)) ([7527a71](7527a71)) * **storage:** say when an unapplied migration is the expected outcome ([#1700](#1700)) ([d83a0a3](d83a0a3)) ### Dependencies * bump the uv-majors group across 2 directories with 1 update ([#1590](#1590)) ([5c86e9d](5c86e9d)) </details> <details><summary>plugin: 2.23.2</summary> ## [2.23.2](plugin-v2.23.1...plugin-v2.23.2) (2026-09-23) ### Bug Fixes * **memory:** make caller ownership of summary/tags durable on the row (09/02 L-08) ([#1693](#1693)) ([6cce76a](6cce76a)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Signed-off-by: release-please[bot] <release-please[bot]@users.noreply.github.com> Signed-off-by: Eldad Caura <eldad@caura.ai> Co-authored-by: caura-deploy-bot[bot] <265395343+caura-deploy-bot[bot]@users.noreply.github.com>
Closes the first half of audit row
ax-0917-m-13— "Documents store text underdatawhile memories usecontent; a naive{title,content}body 422s with 4 missing fields." The second half (unmatched routes returning a bare{"detail":"Not Found"}) shipped in #1636.The friction
An agent that has already used
POST /memorieswrites its payload ascontent. Sent toPOST /documents, that body comes back as the generic SAFE-01 unknown-field 422 fortitleandcontent, plusField requiredforcollection,doc_idanddata— three fields it has never been told about. The response names everything wrong with the request and nothing about what a right one looks like. (tenant_iddropped off that list in #1634; the shape half is what remained.)The design, and what I rejected
The message is the fix, because the naive body cannot be made to succeed. The two fields it is missing are the two that cannot be invented:
doc_idserver-side(collection, doc_id). A minted id silently converts it into create-every-time: the same call twice leaves two rows, a retry after a timeout duplicates instead of converging, and the C34 shrink guard inpostgres_service.document_upsert— which compares a write against the row it is about to replace — has nothing to compare against. A change to what the endpoint means, sold as a convenience.collectiondoc_idinside it is the upsert key. Two callers who both accept the default and pick the same obviousdoc_idoverwrite each other's document.content→datacollectionanddoc_idare still required, so the request still fails and still needs to be told the shape. It buys a second spelling for the payload — plus a precedence rule for bodies sending both — on a store whose own history says extra spellings of a body field cannot be made correct (CAURA-717 removed exactly this kind of guessing fromdoc_indexing).A
mode="before"validator onDocWriteRequestfires when the body borrows a payload name from the memories surface (content,text,body,title,summary,memory_type,metadata) and answers with the document shape in one response: where the payload goes, what the(collection, doc_id)pair means, which fields this body is missing, a minimal body that can be sent as-is, the consequence of omittingdata["summary"], and — whencontentis present — a pointer atPOST /memoriesin case that was the store the caller wanted. Same move as theNO_SUCH_ROUTE404 from #1636, which answers a wrong path by naming the real ones instead of just "not that".The list is named, not a category — matching
SERVER_OWNED_MEMORY_FIELDSand its C6 validator, which is the existing precedent for replacing a generic 422 with an explanatory one. Any other unrecognised key is still just a typo and keeps the ordinary SAFE-01 unknown-field envelope,details.unknown_fieldsincluded.Also describes
collection/doc_id/datain the published schema, wheredatawas an undescribedobject— as informative as the field name. A caller reading the contract had to guess too; the 422 should be the fallback, not the documentation.Compatibility
No shape change. Every request whose behaviour changes here was already a 422 and still is — only the message differs — so no existing caller is affected. Nothing is newly required and no field was removed.
Broker contract: unchanged.
python core-api/scripts/gen_broker_openapi.pyregeneratesopenapi.broker.jsonbyte-identical (--check→ "broker OpenAPI baseline up to date (8 operations)"); documents are not broker operations.Tests
tests/test_ax_m13_document_body_shape.py, 8 tests, all asserting on real HTTP request/response behaviour through the app.Revert test (fix reverted, tests kept, on the committed blob) — 4 failed, 4 passed:
The 4 that pass on both sides are the deliberate no-change guards, and they are the point of the design: a top-level
contentis still refused rather than folded intodata; an ordinary typo still gets the unknown-field envelope; payload keys insidedatastill write; and(collection, doc_id)is still idempotent — writing the pair twice leaves one row with the second payload, which is exactly what a minteddoc_idwould have broken.Selection
-k "document":main, same selection151 = 143 + the 8 new. Zero failures on either side; the failure sets are identical (both empty).
One flake surfaced on a later branch re-run —
test_ph3_storage_documents_fleet.py::test_fleet_in_flight_deploy_false_when_older_or_none— and passes alone on both this branch andmain(other agents share the local postgres). It is about fleet in-flight deploy timestamps and touches nothing in this change.Also green:
test_unknown_field_rejection.py,test_ax_m12_tenant_from_credential.py,test_mcp_doc.py,test_api_documents_skills.py— 162 passed.uvx ruff@0.16.2 checkandformat --checkclean on both files;mypyreports no errors inroutes/documents.py(the 142 errors it prints are pre-existing, in 19 other modules).🤖 Generated with Claude Code