Skip to content

test(OMN-19968): usage-by-model-day and metering-summary give equal rows on SQLite and Postgres - #3148

Merged
jonahgabriel merged 4 commits into
devfrom
lakshman/OMN-19968-parity-readback
Oct 1, 2026
Merged

jonahgabriel merged 4 commits into
devfrom
lakshman/OMN-19968-parity-readback

Conversation

@Patel230

@Patel230 Patel230 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

OMN-19968 asks that writers which run on a developer machine produce the same rows on SQLite and on Postgres. #3067 proved it for two writers. This adds the same proof for the other two writers that have SQLite tables, usage_by_model_day and metering_summary, so a local run and a lab run cannot silently disagree on the Usage page and the Overview savings figures.

What changed

One file, tests/test_omn19968_writer_store_parity.py (+154/-1). Two new cases, test_store_neutral_rows_equal_on_sqlite_and_postgres[usage_by_model_day] and [metering_summary]. Each runs the same store-neutral function once on the real SqliteDatabaseAdapter and once on the real PostgresSyncProjectionAdapter over the same fixtures (apply_usage_call over the ten-call fixture of tests/test_omn19978_usage_by_model_day.py; store_rows over the fold of a three-record request), compares the normalized rows, and asserts no Postgres-only SQL ($n, ::type, jsonb) is seen on the SQLite path. The node migration is rewritten from public. into the throwaway schema so the proof never touches a shared schema.

One helper fix in the same file: the normalizer handled datetime but not date, so a Postgres DATE column and SQLite's ISO text compared unequal. That was the first red run, a gap in the test helper and not a row difference.

No source file changes.

How it was verified

  • uv run pytest tests/test_omn19968_writer_store_parity.py -v at the branch head: 8 passed, 0 skipped (6 before). PostgreSQL 16 was a disposable local cluster built by the test's own fixture.
  • First run, before the normalizer fix: 7 passed, 1 failed (usage_by_model_day, usage_day string versus date).
  • Mutation proof on src/omnimarket/projection/sqlite_database.py _encode, worktree only, restored after each. Baseline: 8 passed. Rounding Decimal to two places: 1 failed (usage_by_model_day). Truncating strings over 10 characters: 4 failed (delegation_events, llm_call_metrics, metering_summary, usage_by_model_day). After restoring: git diff on the source file empty, 8 passed. Rounding missed llm_call_metrics, whose cost is a float that survives rounding at the compared precision.
  • ruff format and ruff check clean; every pre-commit hook passed; the commit is signed.
  • wf pr selfcheck: branch name, no AI attribution, all commits signed, 155 changed lines, no core.hooksPath override all pass.
  • Overlap: wf pr checked this one changed path against 6 open omnimarket PRs and found no overlap. My own earlier read listed 8 open PRs and none touched the file; the two counts differ and I did not reconcile why. feat(OMN-20154): provider quota state is a durable projection that routing and the judge read, every metered call is observed, and 1302 is a capacity cooldown #3134 lists 99 files, at the 100-file cap, so its list may be truncated.

Not proven here: the async Postgres runtime writers (asyncpg pool, advisory lock) are covered only by their own real-Postgres tests, which skip without INTEGRATION_POSTGRES_PASSWORD and which I did not run. The Postgres leg ran on a local cluster, not on the lab host.

Failure paths

n/a: this change adds tests only and writes no receipt, verdict or report.

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.

Every downstream proof — receipts, dod_verify, the evidence chain — assumes the local gates ran,
and the PR body is the only place that assumption is written down. core.hooksPath is the silent
case (OMN-16725): in a worktree .git is a file, the override resolves to nothing, git finds zero
hooks, and the commit succeeds with no warning.

Not in this change

  • node_projection_tenant_credentials has no SQLite table and still uses Postgres-only SQL. It is not covered here; the proposal to carry it onto OMN-19985 is with Jonah on OMN-19968, not decided.
  • The ticket's acceptance criteria are not closed by this PR. The evidence companion and any criterion binding come after merge.
  • No change to the writers themselves, and no change to the async runtime writers.

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

…ows on SQLite and Postgres

#3067 proved parity for two writers only. The other two writers that have SQLite
tables reach the store through a DatabaseAdapter function, so the same function now
runs on the real SQLite adapter and the real Postgres sync adapter over the same
fixtures. The first run exposed a gap in the test normalizer, which handled datetime
but not date, so a Postgres DATE and SQLite's ISO text compared unequal; fixed there.

Mutating the SQLite adapter's encoding turns the new cases red (truncating strings:
all four store cases; rounding Decimal: usage), and restoring it turns them green.
@github-actions

github-actions Bot commented Oct 1, 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 1, 2026 •

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 10
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 pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 1, 2026
…anion for OmniNode-ai/omnimarket#3148

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head bb2ba08d6ed6f148f34e188becc76f38b261c67f.
onexbot-occ-writer Bot added a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 1, 2026
…nimarket#3148

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/36797442926
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 1, 2026
…w for OmniNode-ai/omnimarket (#11990)

* evidence(OMN-19968, OMN-17099, OMN-20154, OMN-20157): author OCC companion for OmniNode-ai/omnimarket#3148

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

* evidence(OMN-19968, OMN-17099, OMN-20154, OMN-20157): self-bind OCC#11990 + rebind contract_sha256

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

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/36797442926

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
@jonahgabriel
jonahgabriel merged commit 66af321 into dev Oct 1, 2026
143 checks passed
@jonahgabriel
jonahgabriel deleted the lakshman/OMN-19968-parity-readback branch October 1, 2026 01:41
Patel230 added a commit that referenced this pull request Oct 3, 2026
…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.
@Patel230

Patel230 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

No reviewer left a comment here. The hostile reviewer's 10 findings were all below quorum and posted no threads; they were style points such as the inline __import__("datetime") calls, and no change was made for them.

Merged as 66af321 on 2026-10-01 for OMN-19968. It changed two files: the store parity test (tests/test_omn19968_writer_store_parity.py) and an import-order fix in benchmarks/delegation_false_pass_growth/readout.py. Every check on the final head passed or was skipped.

Left on the ticket: omnimarket#3183, the llm-cost writer fix, open with every required check green and waiting for review and merge.

Thank you!

jonahgabriel pushed a commit that referenced this pull request Oct 4, 2026
…cabulary on SQLite and Postgres (#3183)

* fix(OMN-19968): the llm-cost writer speaks the shared usage-source vocabulary 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.

* test(OMN-19968): repo-owned evidence contract binding AC1 and AC2 to 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.

* test(OMN-19968): bind AC1 to the usage-source vocabulary test, which 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.

* fix(OMN-19968): pin omnibase-infra 0.38.63, the first release that vendors 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

* fix(OMN-19968): the workflows check out omnibase_infra at the locked 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
Patel230 added a commit that referenced this pull request Oct 5, 2026
…rite lock (#3370)

* fix(OMN-19968): the llm-cost writer speaks the shared usage-source vocabulary 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.

* test(OMN-19968): repo-owned evidence contract binding AC1 and AC2 to 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.

* test(OMN-19968): bind AC1 to the usage-source vocabulary test, which 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.

* fix(OMN-19968): pin omnibase-infra 0.38.63, the first release that vendors 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

* fix(OMN-19968): the workflows check out omnibase_infra at the locked 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

* fix(OMN-19968): a read through the SQLite adapter no longer takes a write 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

* chore(OMN-19968): bump project.version to 0.4.293, past published v0.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

* fix(OMN-19968): drop the connect-time relabel that the dev merge kept 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

* test(OMN-19968): bind the read-lock tests in the repo contract, which 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

---------

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.

2 participants