Skip to content

fix(OMN-19968): a read through the SQLite adapter no longer takes a write lock - #3370

Merged
Patel230 merged 23 commits into
devfrom
lakshman/OMN-19968-sqlite-read-lock
Oct 5, 2026
Merged

Patel230 merged 23 commits into
devfrom
lakshman/OMN-19968-sqlite-read-lock

Conversation

@Patel230

@Patel230 Patel230 commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Update 2026-10-05 08:20Z (head 3e8f6deb7)

Summary

Follow-up to omnimarket#3183 for OMN-19968. #3183 moves old SQLite llm_call_metrics.usage_source labels onto the shared vocabulary with an UPDATE that runs on every connection, reads included. An UPDATE takes a write lock even when it matches nothing, so with #3183 every read through the SQLite adapter waits behind any writer (or fails with database is locked), and a read-only store raises. This change runs the relabel once per store, recorded, so a read never writes.

Stacked on #3183. This branch starts at #3183's head 0b52ce7aa and merges dev. Until #3183 merges, the diff against dev includes #3183's changes; after it merges, a merge of dev leaves only this fix. #3183 itself is untouched.

What changed

  • src/omnimarket/projection/sqlite_database.py: the relabel is now a one-time store step, the SQLite counterpart of migration 0003 (which renames the Postgres enum once). A new bookkeeping table omnimarket_sqlite_store_steps (step, applied_at) records it. On connect:
    • if the step is recorded, nothing is written;
    • if llm_call_metrics has no usage_source column, nothing is written or recorded;
    • otherwise one BEGIN IMMEDIATE transaction creates the step table, relabels, records the step and commits; any error rolls back all of it;
    • a read-only store (primary code SQLITE_READONLY, extended codes masked) is read as it is, with one warning naming the file; every other error, a busy store included, propagates.
  • tests/unit/test_omn19968_usage_source_vocabulary.py: 12 new cases, written before the fix and listed by failure mode. fix(OMN-19968): the llm-cost writer speaks the shared usage-source vocabulary on SQLite and Postgres #3183's reconnect case now seeds a store without the step, which is what "rows written before the vocabulary move" means once the step exists.

Why not a look-first read. An earlier version checked for retired labels with SELECT 1 FROM llm_call_metrics ... before updating. The registry-projection reconcile hook (OMN-17446) refuses that: sqlite_database.py is shared by node packages that do not own llm_call_metrics, and a shared module that decides from another node's time-retained relation has the registry shape the hook exists to catch. Here the decision reads only the adapter's own step record. The hook, its allowlist and every contract.yaml are unchanged.

Stated limit, asserted in test_a_retired_label_written_after_the_step_stays_as_written: a retired label that an older omnimarket writes after a store has recorded the step stays as written. Postgres has the same one-time shape, where the renamed enum refuses such a label.

Not done here: Postgres SQL (unchanged), omnidash's hasRealUsage(), the real-PostgreSQL parity-file hardening.

How it was verified

All local, 2026-10-04, SQLite 3.50.4 in the repo venv. Head 0aa64de88, which is #3183's 0b52ce7aa plus a signed merge of dev 8ee3d6103 (3884aa85d) plus the fix.

  1. RED, then GREEN. On fix(OMN-19968): the llm-cost writer speaks the shared usage-source vocabulary on SQLite and Postgres #3183's unchanged source the new cases give 7 failed, 22 passed. The read-lock case fails with database is locked and the read-only cases with attempt to write a readonly database, both raised by fix(OMN-19968): the llm-cost writer speaks the shared usage-source vocabulary on SQLite and Postgres #3183's per-connection UPDATE. With the fix: 29 passed, also at 0aa64de88.

  2. Mutations, each restored and checked by sha256 afterwards:

    • drop the step check, so the relabel runs on every connect: 3 failed
    • commit the relabel before the step record: 1 failed (the atomicity case)
    • drop the read-only code mask: 1 failed (read-only folder)
    • drop the usage_source column guard: 1 failed
    • treat every error as read-only: 2 failed

    After restoring: 29 passed.

  3. Related suites on the same dev sha 8ee3d6103. These are the 23 other test files that use the SQLite adapter, tests/unit/projection, the llm-cost node tests, the registry gate's tests and the local-store inference test, with -m "not integration and not kafka". Dev: 859 passed, 2 skipped. Branch: 859 passed, 2 skipped.

  4. OMN-17446 hook, scripts/ci/check_registry_projection_reconcile.py: on the branch, OK, exit 0. As a positive control, the same hook on the branch with the step check replaced by the earlier SELECT 1 FROM llm_call_metrics ... fails naming llm_call_metrics, exit 1. The hook, its allowlist and every contract.yaml show 0 diff lines against dev.

  5. mypy --strict on the changed module: no issues. ruff check and ruff format --check: clean. The commit ran every pre-commit hook: 54 passed, 0 failed, the rest "no files to check".

Not run: the integration-marked tests, and the parity file's real-PostgreSQL leg (no Postgres code changed).

Failure paths

n/a: this change writes no receipt, verdict or report. Its own failure paths (read-only file, read-only folder, busy store, a failed step record, a table without the column) are each a test above.

Open defects

none

Local gates

  • No gate was bypassed — no --no-verify, no SKIP=, no --no-gpg-sign, no core.hooksPath override. Every pre-commit and pre-push hook ran.
  • Any hook that failed was fixed at the input, not worked around.

Not in this change

Overlap-Reviewed: #3183 — this branch is stacked on omnimarket#3183 and contains its commits; #3183 lands the shared vocabulary and this PR removes the write lock its per-connection relabel puts on every read, so both are needed.
Overlap-Reviewed: #3319 — omnimarket#3319 reconciles delegation_events columns on connect and this PR replaces the llm_call_metrics relabel; different tables and code, the two heads merge cleanly, so both are needed.

  • The lab parity run on PostgreSQL: this diff touches no Postgres code. The ticket's lab readback for the Postgres leg is the 2026-10-01 run on the lakshman lane (8 of 8, cited on OMN-19968); it was not repeated for this PR.

Overlap-Reviewed: #3371 — omnimarket#3371 (OMN-19976) also edits src/omnimarket/projection/sqlite_database.py, only _ensure_columns' locking; this PR changes the one-time usage-source relabel step on connect. git merge-tree of the two heads is clean (2026-10-05 06:59Z).

Overlap-Reviewed: #3378 — omnimarket#3378 (OMN-20006) edits the usage-table DDL and the connect-time reconcile in the same file, and commits the transaction #3183's relabel left open; this PR removes that connect-time relabel, so #3378's commit becomes a no-op rather than a conflict. git merge-tree is clean (06:59Z); whichever merges second re-runs both PRs' tests.

Overlap-Reviewed: #3406 — omnimarket#3406 (OMN-20009) adds metering_summary.savings_per_measured_run_usd to the DDL in the same file; this PR touches only the relabel step. git merge-tree is clean (06:59Z).

Evidence-Ticket: OMN-19968
Evidence-Source: OCC#12812

…cabulary on SQLite and Postgres

node_projection_llm_cost wrote the retired labels API/ESTIMATED/MISSING, while
omnibase_infra migration 077 (OMN-10382) moved the shared usage_source_type to
measured/estimated/unknown (EnumUsageSource). On any database infra has migrated,
0001's typname guard matched the shared type and created nothing, so every write
of 'MISSING' or 'API' was refused; the ticket's own lab step failed 6 passed,
2 failed on the lakshman lane at 07:46Z.

- row_llm_call_metrics maps provider-reported usage to measured, ESTIMATED to
  estimated, and MISSING/UNKNOWN/absent/unrecognised to unknown (retired labels
  folded in, case-insensitive).
- The SQLite schema defaults to 'unknown', and existing SQLite rows are moved
  onto the shared vocabulary in place.
- Migration 0003 renames an omnimarket-created usage_source_type's labels in
  place (RENAME VALUE, no table rewrite), resolving the type through
  search_path; a no-op where the shared vocabulary is already present.
- Tests and fixtures use the shared values; a new test file pins the mapping,
  the SQLite reconciliation and the migration's uniqueness.
…the parity tests

The repo-evidence gate (OMN-20070) requires a PR that cites OMN-19968 to
carry contracts/OMN-19968.yaml, with every acceptance criterion bound by
binds_ac to a test_passes item naming an existing test file.

AC1 binds to the rows_equal parity tests in
tests/test_omn19968_writer_store_parity.py (the four writers, real SQLite
adapter against real Postgres; the Postgres half skips without a Postgres).
AC2 binds to the postgres_only_sql tests (a recording SQLite connection
refuses $n placeholders, ::jsonb and enum casts on every writer's path).

node_dod_verify --execution-audience hosted on this head: PASS, 2 of 2
verified, both behavior-proving.
…fails at the merge base

The repo-evidence gate overlays a PR's tests on the merge-base source and
requires every bound test to fail there. The parity tests already pass on
dev (they were delivered by #3067 and #3148), so binding them read as
always-pass. AC1 now binds to tests/unit/test_omn19968_usage_source_vocabulary.py,
the test this change adds: against the merge-base source (2c6dabe) it
fails 17 of 17, on this head it passes. AC2 is not changed by this PR and
is no longer claimed here.
…ndors migration 0003

The Sibling Release Compatibility Gate (OMN-20381) refused
node_projection_llm_cost/migrations/0003_usage_source_shared_vocabulary.sql:
it reads the omnibase-infra pin from uv.lock and requires every node
migration a change adds to be byte-identical in that release. The pin was
0.38.61, which lacks it; omnibase_infra#4420 (98620a57) vendored it and is
first released in v0.38.63.

Raise the floor to >=0.38.63,<0.39.0 in dependencies and
override-dependencies, and refresh uv.lock for omnibase-infra only (uv
0.12.22, the version CI installs: lock revision stays 5, 10 lines change,
no other package moves). The gate's comparison run locally: v0.38.61 lacks
0003, v0.38.63 is byte-identical.

Onex-Lane: omn19981-evidence
Onex-Session: bf37a08791cf43ccab41e737a008f469
…v0.38.63

tests/ci/test_sibling_pins_omn20001.py::test_infra_checkout_pins_match_lock
failed on this PR's CI (Tests split 2/20, run 37138844983): six workflows
still checked out omnibase_infra at v0.38.61 after the lock moved to
0.38.63. Move every such checkout, and the messages that name the version,
to v0.38.63, and the one test that pins the overlay-parity checkout ref.
These are the same edits omnimarket#3309 makes for the same pin.

tests/ci, tests/seam_goldens and the pricing tests on this branch and on
dev at the merge base 76e1673 fail the same 23 tests locally (release
staleness and others; identical sets, environmental here) and nothing else.
The pin test passes; restoring one v0.38.61 ref fails it.

Onex-Lane: omn19981-evidence
Onex-Session: bf37a08791cf43ccab41e737a008f469
…ge-source-vocab

dev raised the omnibase-infra floor to 0.38.64 (omnimarket#3293), past the
0.38.63 this branch pinned for migration 0003. Nine files conflicted on that
one value (pyproject.toml, uv.lock, six workflows and one CI test); each
takes dev's version, so this branch no longer changes any of them.

Onex-Lane: omn19968-usage-vocab
Onex-Session: b4a9c00fa6f045f4b54fca18b2cdd0e7
dev reopened as 0.4.291 (#3367); the release-identity hook refuses a src
commit on #3183's published 0.4.286, so the follow-up builds on dev.

Onex-Lane: omn19968-readlock-followup
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
…rite lock

#3183 moved old usage_source labels onto the shared vocabulary with an
UPDATE run on every connection. An UPDATE takes a write lock even when it
matches nothing, so every read waited behind any writer (or failed with
'database is locked') and a read-only store raised.

The relabel is now a one-time store step, like migration 0003 on
Postgres: it runs once, in one transaction with its record in
omnimarket_sqlite_store_steps, and a store that has recorded it opens
with reads only. A read-only store is read as it is, with one warning;
any other error propagates.

The decision reads the adapter's own step record, not llm_call_metrics.
A look-first read of that table was refused by the OMN-17446 registry
hook: this module is shared by nodes that do not own the table, and a
shared module deciding from another node's time-retained relation is the
shape that hook guards against.

Onex-Lane: omn19968-readlock-followup
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
@Patel230
Patel230 requested a review from jonahgabriel October 4, 2026 12:11
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

Rule Description
ARCH-TOPIC-001 No hardcoded Kafka topic strings in handler code
ARCH-DI-001 No event_bus=None bypass
ARCH-DI-002 No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003 No reinvented DI containers
ARCH-TOPIC-002 contract.yaml topics follow `onex.{cmd

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 12
Models succeeded: qwen3-review,gpt-oss-review


Gate semantics

Verdict Meaning Blocks merge?
passed >=2 models succeeded, no critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Fewer than 2 models succeeded (infra unavailable/timeout) Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

onexbot-occ-writer Bot added a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 4, 2026
…nimarket#3370

The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited.

Ticket: OMN-16859 (AC3b)
Onex-Workflow: OCC Receipt Runner
Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/37211600570
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 4, 2026
)

* evidence(OMN-19968): author OCC companion for OmniNode-ai/omnimarket#3370

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 0aa64de88a3b2d048c9e4fa24d41b365b81f573e.

* evidence(OMN-19968): self-bind OCC#12812 + rebind contract_sha256

* evidence(OMN-16859): executed test_passes receipts for OmniNode-ai/omnimarket#3370

The declared checks were executed for real in the OmniNode-ai/omnimarket checkout at the PR head by the product-repo OCC receipt runner, and the results written append-only into this companion. Producer-minted PENDING receipts are superseded by a net-new record; no existing receipt was edited.

Ticket: OMN-16859 (AC3b)
Onex-Workflow: OCC Receipt Runner
Onex-Run: https://github.com/OmniNode-ai/omnimarket/actions/runs/37211600570

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
Patel230 and others added 6 commits October 4, 2026 21:23
…4.292

The release identity gate (OMN-16344) refuses packaged source changes on a version that is already published. v0.4.292 was released 2026-10-04T15:18Z; this matches the open bot bump omnimarket#3375 (pyproject.toml and uv.lock, identical lines).

Onex-Lane: ci-fix2-OMN-19968
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
pyproject.toml and uv.lock conflicted on the version line only (dev 0.4.292, branch 0.4.293); kept 0.4.293, past published v0.4.292 for the release identity gate.

Onex-Lane: ci-fix2-OMN-19968
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
Brings in #3375 (dev at 0.4.293); the branch's own version commit is now identical to dev.

Onex-Lane: ci-fix2-OMN-19968
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
Takes dev's version line; 0.4.293 is published.

Onex-Lane: claude-d99c149d
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
…ad-lock

Onex-Lane: claude-d99c149d
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
Bring the branch up to date after omnimarket#3183 (the usage-source vocabulary fix this branch started from) landed on dev as a squash, which left the same hunks conflicting.

# Conflicts:
#	tests/unit/test_omn19968_usage_source_vocabulary.py
… beside the recorded step

omnimarket#3183 landed on dev with the relabel inside _reconcile_legacy_llm_call_metrics, which runs on every connect and so takes a write lock on every read. This branch moves it into the once-per-store step; git merged both, so the old UPDATE came back and six of the read-lock tests failed on the merged tree. Keep only the step.

Dev is at 0.4.299, which is already published, and this change touches packaged source, so project.version moves to 0.4.300 (release-identity gate, OMN-16344).

Onex-Lane: omn19981-evidence
Onex-Session: bf37a08791cf43ccab41e737a008f469
… the merge base's contract does not carry

repo-evidence / dod-verify refused 0ebc5ef: after #3183 merged, contracts/OMN-19968.yaml on dev binds only the usage-source vocabulary check, so every check this PR bound was "carried unchanged from the merge base" and the PR bound no evidence of its own (run 37257169127). Add a check on the read-lock tests; the selected six fail on origin/dev cce192a and pass here.

Onex-Lane: omn19981-evidence
Onex-Session: bf37a08791cf43ccab41e737a008f469
…e-read-lock

dev opened v0.4.301 after v0.4.300 was published, so the 0.4.300 version lines
on this branch conflicted. Both version lines now take dev's 0.4.301, which is
ahead of the published v0.4.300; nothing else conflicted.

Onex-Lane: omn19968-sqlite-read-lock
Onex-Session: e72e9157-677b-4b13-bd77-daa2be198c16
…-lock

dev moved while this branch was being brought current; the second merge is
clean, so no file needed a resolution.

Onex-Lane: omn19968-sqlite-read-lock
Onex-Session: e72e9157-677b-4b13-bd77-daa2be198c16
@Patel230
Patel230 merged commit 2c529ce into dev Oct 5, 2026
152 checks passed
@Patel230
Patel230 deleted the lakshman/OMN-19968-sqlite-read-lock branch October 5, 2026 18:10
@Patel230

Patel230 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Codex merge close-out — OMN-19968 / omnimarket#3370

  • The live Linear ticket and all comments were re-read before merge; no unresolved request specific to this PR.
  • Live GitHub review pinned to head 0f9c686: merge state CLEAN, no conflicts, 148 successful checks, 3 skipped, 1 neutral, 0 failing, and 0 unresolved review threads.
  • AC1: the SQLite adapter read path no longer acquires a write lock; the one-time usage-source relabel is recorded per store.
  • AC2: no PostgreSQL-only SQL is reachable through the SQLite writer paths.
  • Author handoff evidence at the reviewed head: 526 passed / 2 skipped plus 386 passed; Ruff, format, and mypy strict clean.
  • Squash-merged as merge 2c529ce at 2026-10-05T18:10:25Z.

Ticket status is unchanged; Lakshman decides Done.

Thank you!

Patel230 added a commit that referenced this pull request Oct 6, 2026
…ore step

#3370 (OMN-19968) turned the llm_call_metrics relabel into a one-time
store step: it runs in its own write transaction that commits it with its
row in omnimarket_sqlite_store_steps, a store that has recorded it opens
with reads only, a read-only store is read as it is with a warning, and a
busy store raises. This PR's usage reconcile still ran as its own
connect-time write, outside that mechanism, so a read-only store written
before this change raised "attempt to write a readonly database" on every
read of every table.

The reconcile is now the store step omn20006_usage_by_model_day_measured_cost
(the SQLite counterpart of usage_by_model_day migration 0002). _connect
runs every pending step through one _apply_store_steps: each step's
columns or relabel and its record commit together, the columns are read
again under the step's write lock so racing first opens add each once, a
read-only store gets one warning naming every step it lacks, and any other
error raises. The schema-probe check and the in_transaction commit guard
are gone; the guard was dead once #3370 moved the relabel out of the
connect-time DDL, and its comment described code that no longer exists.

Tests: tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py
(five fail on the previous head). The OMN-19968 vocabulary tests read only
their own step's record, since the store now records a second step.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4
Patel230 added a commit that referenced this pull request Oct 7, 2026
…timates (#3378)

* feat(OMN-20006): usage-by-model-day keeps measured cost apart from estimates

The Usage page must count runs whose cost was not measured and leave them
out of the cost it shows (AC3), and must never show 0 for a cost nobody
measured (AC4). The projection stored every call's cost as a number and
had no idea how that number was obtained, so no page could do either.

Each call now carries its usage_source (unknown when the event does not
say, so an estimate is never taken for a measurement). Each day-and-model
row serves measured_cost_usd, the sum over measured calls and NULL when
there were none, and unmeasured_call_count. cost_usd and call_count keep
their meaning. Migration 0002 is additive; old rows read unknown/NULL/0.

The exposure now declares tenant_column, so the server scopes a read to
the reading tenant instead of the page filtering rows in the browser.
The local onex delegate path writes the usage rows too, keyed by the
run's correlation id and dated by the terminal's emitted_at; a failure
there is logged and swallowed like the call-metrics write.

A local store also lacked projection_cursor, so a served read of this
exposure answered 503 projection_column_missing on every local install.
The store now adds the missing columns on connect and stamps the cursor
on each insert and recount from a one-row sequence table, as Postgres's
BIGSERIAL does from its sequence.

The consumers opt-out is deleted: the omnidash Usage page is the reader.
Version 0.4.294: packaged source changed (0.4.293 is published).

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): shorten the AC1 dod_evidence id to the 50-character limit

The ticket_contract model caps dod_evidence ids at 50 characters. The AC1
id was 51, so contract-validation failed on the PR before any check ran.
The id is a label only; nothing else references it.

Onex-Lane: omn20006-ci
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* test(OMN-20006): unpack the tenant-scoped exposure by length, not by tuple target

CodeQL flagged the single-target tuple unpack of the loader's result as a
length mismatch (alert on tests/test_omn19978_usage_by_model_day.py:386) and
failed the PR's CodeQL check. The sibling test already asserts the length and
indexes, so this one does the same and still fails if the loader returns two
exposures.

Onex-Lane: omn20006-ci
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): promote ModelUsageCallEvent to omnimarket.models so the dispatch port stops reaching into a sibling node

The local delegation dispatch port imported ModelUsageCallEvent from
node_projection_usage_by_model_day.models, tripping
test_no_new_cross_node_model_reach_ins. The model now lives in
omnimarket.models.model_usage_call_event; the node's module re-exports it.

Onex-Lane: omn20006-reachin
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* chore(OMN-20006): bump to 0.4.300, past the published 0.4.299

After merging dev, v0.4.299 is reachable from this branch and dev itself
reads 0.4.299, so the release-identity gate (OMN-16344) refuses a src/
change at that version: check_release_identity.py --base origin/dev exited
1 at the merge commit and 0 after this bump. 0.4.300 is the number the other
open omnimarket PRs and the post-release bump PR already use.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): an existing local store opens again after the dev merge

Dev's llm_call_metrics relabel (#3183) runs an UPDATE on every connect
once that table has usage_source, which every store written by the
current release does. The sqlite3 module opens a transaction before
that UPDATE and leaves it open until _connect commits at the end. The
usage-by-model-day reconcile sits between the two and issues BEGIN
IMMEDIATE, so on such a store it raised "cannot start a transaction
within a transaction": the first open after upgrading failed, and
concurrent first opens (dev's #3319 test) failed 4 of 5 runs.

The reconcile now commits that open transaction before taking the
write lock. The relabel is idempotent and _connect committed it anyway,
so nothing is lost; the new test asserts the relabel is kept, and a
rollback in place of the commit fails it.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): run the usage-by-model-day reconcile as a one-time store step

#3370 (OMN-19968) turned the llm_call_metrics relabel into a one-time
store step: it runs in its own write transaction that commits it with its
row in omnimarket_sqlite_store_steps, a store that has recorded it opens
with reads only, a read-only store is read as it is with a warning, and a
busy store raises. This PR's usage reconcile still ran as its own
connect-time write, outside that mechanism, so a read-only store written
before this change raised "attempt to write a readonly database" on every
read of every table.

The reconcile is now the store step omn20006_usage_by_model_day_measured_cost
(the SQLite counterpart of usage_by_model_day migration 0002). _connect
runs every pending step through one _apply_store_steps: each step's
columns or relabel and its record commit together, the columns are read
again under the step's write lock so racing first opens add each once, a
read-only store gets one warning naming every step it lacks, and any other
error raises. The schema-probe check and the in_transaction commit guard
are gone; the guard was dead once #3370 moved the relabel out of the
connect-time DDL, and its comment described code that no longer exists.

Tests: tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py
(five fail on the previous head). The OMN-19968 vocabulary tests read only
their own step's record, since the store now records a second step.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): book each delegation attempt's usage to its own model

The local path wrote one usage call per run: the final model, the final
attempt's tokens and the run's cumulative cost, always as measured. On an
escalated run that put every rejected attempt's spend on the model that
answered, so the Usage page's cost per model was wrong for that run even
though the total was right, and an attempt whose cost was not measured was
counted as measured because the last one was.

The dispatch loop now records what each banked call consumed (model,
tokens, cost, usage source) at the same two points it adds to the run's
cumulative cost, so the two cannot drift. Each call that reached a provider
becomes its own usage call with its own usage source; the per-model sum
still equals the delegation row's cost_usd. A call that never reached a
provider carries no tokens and no cost and is not a usage call. A
single-call run keeps the correlation id as its key; several calls take
<correlation id>:<n> in attempt order.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): usage tokens follow metering's one definition

AC1 binds the Usage page's token totals to onex metering --json, which
reads delegation_events and so counts only the deciding attempt's
tokens. Booking every attempt's tokens made an escalated run's usage
tokens larger than metering's for the same window, which is AC1's
falsifier.

Cost stays split per attempt and per model; tokens now go on the
deciding attempt's call only, and earlier attempts' calls carry 0. The
token columns are NOT NULL on both stores, so 0 cannot be told apart
from a measured zero without a schema change. Counting escalated-from
attempts' tokens belongs in the metering summary, so the CLI and the
page move together; that is a follow-up, not this change.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* build(OMN-20006): pin omnibase-infra 0.38.67, the first release vendoring usage_by_model_day 0002

The Sibling Release Compatibility Gate (OMN-20381) refuses a node migration
until the pinned omnibase_infra release carries it byte-identical. 0.38.67
(tag 2bbaaeb) vendors 0002_usage_by_model_day_measured_cost.sql via
omnibase_infra#4559; 0.38.66 does not. The lock moves omnibase-infra only.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): the clean-install delegate probe asks onex delegate for --json

omnibase_infra#4609 (OMN-20124, in 0.38.67) prints the plain answer when
stdout is redirected; JSON now comes only with --json. The probe parsed
stdout as JSON without asking for it, so on 0.38.67 every field read None
and the Customer Clean-Install and Sibling Release delegate gates failed on
dev and on every PR. --json exists in 0.38.66 and 0.38.67, so the probe
now works on both: local runs FAIL unchanged on 0.38.67, PASS with --json
on 0.38.67 and on 0.38.66.

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* ci(OMN-20006): every omnibase_infra checkout pins v0.38.67, matching the lock

The 0.38.67 bump moved pyproject.toml and uv.lock but left seven workflow and
test references on v0.38.66, so test_infra_checkout_pins_match_lock failed
(Tests split 2/20). The same files the 0.38.66 bump touched (omnimarket#3417)
now name v0.38.67; tests/ci/test_sibling_pins_omn20001.py and
test_ci_bus_overlay_parity.py pass (29).

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

* fix(OMN-20006): store step rolls back any failure but catches only the read-only one

CodeQL flagged the store step's except block (alert 1545), and our merge rule
needs every review thread resolved. It caught BaseException to roll back, then
re-raised everything except a read-only store. It now catches
sqlite3.OperationalError for the read-only case, and keeps a bare
"except BaseException: rollback; raise" so any other failure, an interrupt
included, still releases the write lock before it propagates. The behaviour is
unchanged. That rollback was never pinned: removing it passed every test. A new
test holds the error and checks another writer can take the lock at once; it
fails with "database is locked" when the rollback is removed. The thread-safety
test now records Exception, not BaseException (alert 1546).

Onex-Lane: omn20006-usage-page
Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4

---------

Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
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.

1 participant