fix(engine): make the in-flight refresh dedupe work on Oracle (#3487)

[chethanuk]

Problem

The sub-bank dedupe in _submit_async_operation binds its JSON key:

AND task_payload->>$7 = $8

Oracle never sees that as JSON. _JSON_ARROW_TEXT_RE (db/oracle.py:82) only matches a quoted literal key, so ->>$7 survives the rewrite as a raw PG operator and the statement raises. Both callers swallow it, so nothing surfaces.

flowchart LR
    C["consolidator.py:1944<br/>skip_if_in_flight=True"] --> S["_submit_async_operation"]
    M["maintenance.py:505<br/>cron refresh"] --> S
    S --> D{"dedupe_in_flight<br/>_payload_key"}
    D -->|None| U["INSERT ... VALUES<br/>fine on both dialects"]
    D -->|set| P{"backend"}
    P -->|postgresql| OK["INSERT ... WHERE NOT EXISTS<br/>fine"]
    P -->|oracle| X["task_payload->>$7 raises,<br/>caught at consolidator.py:1955"]
    X --> L["'Failed to trigger refresh',<br/>model never refreshed"]

Loading

The comment on this branch claimed it was PG-only, and that was true when #3210 wrote it: the maintenance loop is PG-gated at maintenance.py:96. Then #3411 added the post-consolidation flush, and consolidation does run on Oracle. Every consolidation-triggered mental-model refresh on Oracle has been dropped silently since.

Fixes #3487

Fix

Inline the key so the rewrite fires:

AND task_payload->>'mental_model_id' = $7
  → JSON_VALUE(task_payload, '$.mental_model_id') = :7

It is an internal constant, but the code now rejects anything that is not a plain identifier rather than trusting the caller.

Oracle still needs its own write path. INSERT ... SELECT requires a FROM DUAL there and cannot carry a RETURNING clause at all, so the check cannot ride inside the INSERT the way it does on PG. Splitting them is safe because serialize already holds FOR NO KEY UPDATE (FOR UPDATE on Oracle) on the bank row for the whole transaction, so no concurrent submit for the same bank can interleave. PG keeps the single-statement form, where the check holds even without that lock.

No behaviour change on PostgreSQL.

Note on the issue

#3487 reports a PostgreSQL symptom that main already handles. The dedupe has covered status='pending' since #3210, the consolidation flush opted in via #3411, and test_concurrent_scheduled_submits_queue_one_refresh pins it. The reporter's deployment most likely predates those.

I still have this closing #3487, because the issue asks for "at most one pending refresh per (bank_id, mental_model_id)" and that guarantee did not hold on Oracle. Say the word if you would rather it stayed open for the reporter to confirm their version.

Test evidence

Check	Result
tests/test_db_abstraction.py	125 passed
tests/test_mental_model_scheduled_refresh.py	8 passed
ruff check / ruff format / ty check	clean

One new case, test_jsonb_arrow_rewrite_needs_a_literal_key, asserts the emitted predicate rewrites to JSON_VALUE with a literal path, and that a bound ->>$N does not rewrite. The second assertion pins the bug, not just the fix.

It calls the real rewriter instead of mocking a connection. A mock would have passed on the broken SQL too, since no Oracle parser is involved either way.

Limitation

Not introduced here and not addressed: on Oracle, _convert_arg (db/oracle.py:107) coerces any dash-formatted UUID string bind to RAW(16), so a client-supplied UUID-shaped mental_model_id binds as bytes and will not match the VARCHAR2 result of JSON_VALUE. Server-generated ids are mm-<hex> and unaffected, and bank_id in the same query carries the identical exposure.

[nicoloboschi]

superseded by #3550