Repository navigation
fix(models): declare the JSON columns migration 001 creates as json - #1794
Merged
Merged
Conversation
Contributor
|
Claude Code Review — skipped: draft PR |
eldad-caura-ai
marked this pull request as ready for review
October 3, 2026 19:04
Contributor
Claude Code Review ✅ No issues found. Reviewed by |
eldad-caura-ai
force-pushed
the
fix/fleet-command-json-columns
branch
from
October 3, 2026 19:12
7f63c67 to
0e4ffd5
Compare
Seventeen columns created by migration 001 are json in every migrated database, but their models declared JSONB (CAURA-595): memories.metadata, documents.data, audit_log.detail, entities.attributes, agents.search_profile, six analysis_reports columns, four fleet_nodes columns, and fleet_commands.payload and result. SQL written from the models failed only on migrated databases (jsonb_typeof, the ? operator, COALESCE against a jsonb literal). The models now declare postgresql.JSON for these columns, matching the schema the migration chain builds and CI runs against. There is no migration and no data change. A database that create_all built before the chain was authoritative may still hold jsonb, so the existing ::jsonb casts stay. A storage test compares every JSON-family model column with the migrated schema's information_schema. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: eldad-caura <eldad@caura.ai>
eldad-caura-ai
force-pushed
the
fix/fleet-command-json-columns
branch
from
October 3, 2026 19:13
0e4ffd5 to
8308a36
Compare
Member
Author
|
@erni-a plerar parproev |
erni-a
approved these changes
Oct 3, 2026
Merged
eldad-caura-ai
pushed a commit
that referenced
this pull request
Oct 3, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>backend: 3.21.2</summary> ## [3.21.2](backend-v3.21.1...backend-v3.21.2) (2026-10-03) ### Bug Fixes * **graph:** hide graph data mined from soft-deleted memories from every reader ([#1796](#1796)) ([97315d0](97315d0)) * **interview:** admit a fleet node's window only on the request sent to it ([#1792](#1792)) ([4c56b8e](4c56b8e)) * **models:** declare the JSON columns migration 001 creates as json ([#1794](#1794)) ([e0a0a9b](e0a0a9b)) </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> Co-authored-by: caura-deploy-bot[bot] <265395343+caura-deploy-bot[bot]@users.noreply.github.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: the app-wide 422 handler also catches UnicodeDecodeError. A body that is not valid UTF-8 fails before JSON parsing, so every router that parses its body inline answered 500, which core-api retries as a 503. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: the app-wide 422 handler also catches UnicodeDecodeError. A body that is not valid UTF-8 fails before JSON parsing, so every router that parses its body inline answered 500, which core-api retries as a 503. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: the app-wide 422 handler also catches UnicodeDecodeError. A body that is not valid UTF-8 fails before JSON parsing, so every router that parses its body inline answered 500, which core-api retries as a 503. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: a body that is not valid UTF-8 answers 422. It fails before JSON parsing, with UnicodeDecodeError, so every router that parses its body inline answered 500, which core-api retries as a 503. The app-wide handler answers 422 only for a decode raised parsing the body; any other stays a 500. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: a body that is not valid UTF-8 answers 422. It fails before JSON parsing, with UnicodeDecodeError, so every router that parses its body inline answered 500, which core-api retries as a 503. The app-wide handler answers 422 only for a decode raised parsing the body; any other stays a 500. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: a body that is not valid UTF-8 answers 422. It fails before JSON parsing, with UnicodeDecodeError, so every router that parses its body inline answered 500, which core-api retries as a 503. The app-wide handler answers 422 only for a decode raised parsing the body; any other stays a 500. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
L-37: a body that is not valid UTF-8 answers 422. It fails before JSON parsing, with UnicodeDecodeError, so every router that parses its body inline answered 500, which core-api retries as a 503. The app-wide handler answers 422 only for a decode raised parsing the body; any other stays a 500. The local catches in tenant_suppression and preview go. L-139: a reader given READ_DATABASE_URL opens every session there. Only get_read_session followed it; the get_session call sites, about two thirds, still reached DATABASE_URL, the primary on a reader set up from the README. Shutdown disposes every engine the process built, on any role. The README says so. L-145: preflight_012 places the head on the migration chain first. At 012 or later it reports 012 applied, counts nothing and exits 0; it had reported every re-embedded database past 012 destructive. L-97: the models declare nullable the 17 columns migration 001 created nullable, as #1794 did for their JSON types, and a check fails on any nullability drift. The PATCH guard still refuses a null weight. Signed-off-by: eldad-caura <eldad@caura.ai> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
eldad-caura-ai
added a commit
that referenced
this pull request
Oct 11, 2026
## Why The four open findings from audit batch B28 (database upgrade and readiness). All are in the storage service. - **L-37**: a request body that is not valid UTF-8 fails with `UnicodeDecodeError`, which the app-wide 422 handler for `JSONDecodeError` does not catch. Every router that parses its body inline answers 500, which core-api turns into a retryable 503. - **L-139**: a reader configured from the README (`CORE_STORAGE_ROLE=reader`, `READ_DATABASE_URL` set) sends only its `get_read_session` call sites to the read database. The rest, about two thirds, open the writer engine, still on `DATABASE_URL`. Shutdown disposes only the writer engine, and on a reader nothing. - **L-145**: `preflight_012` promises exit 0 for a database already past 012 but decides on the row count alone, so a re-embedded database past 012 is reported destructive (exit 1). - **L-97** (the remaining half; the JSON types were fixed in #1794): 17 columns migration 001 created nullable are declared NOT NULL by their models, so autogenerate proposes `SET NOT NULL` and `create_all` test schemas refuse rows production stores. ## What changes - **L-37**: an app-wide `UnicodeDecodeError` handler answers 422, as the malformed-JSON handler does, but only for a decode raised inside `Request.json`, that is, while parsing the body. Any other decode failure is re-raised and stays a 500. The two routers that caught it locally (`tenant_suppression`, `preview`) now rely on it. - **L-139**: on a reader given `READ_DATABASE_URL`, `get_engine()` returns the read engine, so every session opens there and the reader keeps one pool. This is what `config.py` already describes and what the SaaS reader gets by setting `DATABASE_URL` to the read pool; a reader without `READ_DATABASE_URL`, and the writer and hybrid roles, are unchanged. Shutdown calls a new `dispose_engines()`, which closes every engine the process built, on any role. The README says what `READ_DATABASE_URL` does on each role. - **L-145**: `preflight_012` places the head on the migration chain before counting. At 012 or later it reports 012 applied, counts nothing and exits 0; the JSON report gains `migration_applied`. A head before 012, or one this checkout's chain does not know, is counted as before. - **L-97**: the models declare `nullable=True` on the 17 columns migration 001 created nullable, as #1794 did for their JSON types. Annotations are unchanged. A new `compare_metadata` check fails on any `modify_nullable` diff. `NON_NULLABLE_UPDATE_FIELDS` keeps `weight`, so a PATCH still refuses a null weight; its drift test now says why. The database is not changed. Eldad decided against a NOT NULL migration; a follow-up PR will stop storage's bulk insert writing an explicit NULL `weight` or `created_at`, the one path that stores a NULL today. - `test_schema_drift_report`'s lifespan test now checks that shutdown calls `dispose_engines()`. ## Rollout No migration. Readers given `READ_DATABASE_URL` stop connecting to `DATABASE_URL`, so a reader whose read database is a replica serves every query from it. ## Evidence 1. **Tests alone** (b2520b6, [run 38136238575](https://github.com/caura-ai/caura/actions/runs/38136238575)): the storage suite failed exactly the 9 new cases, each on the gap it pins; 952 passed. - **L-37:** a non-UTF-8 body to `POST /agents` and `/agents/create-only` answered 500. The two routers with local catches already answered 422 and passed. - **L-139:** a reader's `get_session` bound to `primary.invalid`, and shutdown disposed neither engine on any role. The control (writer and hybrid keep the primary) passed. - **L-145:** a database at 065, and one at 012, exited 1. The control (011 with vectors exits 1) passed. - **L-97:** the check listed exactly the 17 columns, each schema NULL and model NOT NULL. 2. **The fix** (5e945fc): passed CI ([run 38137030960](https://github.com/caura-ai/caura/actions/runs/38137030960)), as did Eldad's rebase onto main, 6fd872e ([run 38137217222](https://github.com/caura-ai/caura/actions/runs/38137217222)). 3. **Review round 1:** Claude Code Review found one Medium issue: the `UnicodeDecodeError` handler also answered 422 for decode failures unrelated to the body. Its test alone (7f7db5f, [run 38137284764](https://github.com/caura-ai/caura/actions/runs/38137284764)) failed on that handler with 422; 961 passed. The fix, 03d1b29: _CI pending_. Claude Code Review found no issues on it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: eldad-caura <eldad@caura.ai> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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
Migration 001 creates seventeen columns as
json, but their models declareJSONB(the CAURA-595 drift). Every database built by the migration chain, CI's included, hasjson. Code written from the models fails only there:jsonb_typeof(fleet_commands.result)raisedUndefinedFunctionErrorin caura PR fix(interview): admit a fleet node's window only on the request sent to it #1792's CI;backfill_embeddingshit the same error on the?operator;CannotCoerceErroronCOALESCE(metadata, '{}'::jsonb)until it gained a::jsonbcast.Nothing after migration 001 converts these columns. The existing model-vs-schema test (
test_models_match_the_migrated_schema.py) checks tables and indexes only, so it never noticed.memoriesmetadatadocumentsdataaudit_logdetailentitiesattributesagentssearch_profileanalysis_reportssummary,hygiene,health,usage_data,issues,crystallizationfleet_nodesagents_json,tools_json,channels_json,metadatafleet_commandspayload,resultThe other eleven JSON columns, all added after 001, are
jsonbin both the models and the migrations. They are unchanged.Change
postgresql.JSON, which matches what the migration chain builds and what CI tests.postgresql.JSONkeeps.astext;.as_float()and.as_boolean()come from the generic JSON type. Those three are the only JSON operators the ORM uses on these columns.information_schema.columnsin the migrated schema. It fails on anyjson/jsonbdisagreement, so the next drift fails CI.JSONBnow describe the real situation. That includes the claim filter caura PR fix(interview): admit a fleet node's window only on the request sent to it #1792 added, which comparesfleet_commands.resultas text so it works on both types.Not in this PR (approved choice)
There is no migration. Converting these columns to
jsonbwould rewritememories,documents,audit_logandentitiesunder an exclusive lock, andjsonbalso normalises stored JSON. A database thatcreate_allbuilt and that was stamped before the chain was authoritative may still holdjsonb. So statements on these columns must keep working on both types, which is why the existing::jsonbcasts stay.Verification
model jsonb, schema json, matching the inventory taken from the files. The model change is now amended into the same single commit.One signed-off commit. Do not merge or enable auto-merge; Eldad merges after review.
🤖 Generated with Claude Code