From 2c2ed032cb0334f31e3c41676c9bb6f1b19cd1c1 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sun, 4 Oct 2026 20:34:52 +0530 Subject: [PATCH 01/13] 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 --- contracts/OMN-20006.yaml | 55 +++++ .../ports/port_local_delegation_dispatch.py | 35 +++ .../contract.yaml | 24 +- .../handler_projection_usage_by_model_day.py | 1 + .../handler_usage_by_model_day_store.py | 11 + .../handler_usage_by_model_day_writer.py | 20 +- .../0002_usage_by_model_day_measured_cost.sql | 15 ++ .../models/model_usage_call_delta.py | 3 + .../models/model_usage_call_event.py | 5 + src/omnimarket/projection/sqlite_database.py | 90 ++++++- ...test_omn20006_usage_by_model_day_served.py | 133 +++++++++++ tests/test_omn19968_writer_store_parity.py | 61 ++++- tests/test_omn19978_usage_by_model_day.py | 223 +++++++++++++++++- ...n19978_usage_by_model_day_real_postgres.py | 107 ++++++++- ...test_omn19978_usage_by_model_day_writer.py | 24 ++ .../test_local_dispatch_usage_by_model_day.py | 189 +++++++++++++++ 16 files changed, 958 insertions(+), 38 deletions(-) create mode 100644 contracts/OMN-20006.yaml create mode 100644 src/omnimarket/nodes/node_projection_usage_by_model_day/migrations/0002_usage_by_model_day_measured_cost.sql create mode 100644 tests/projection/test_omn20006_usage_by_model_day_served.py create mode 100644 tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py diff --git a/contracts/OMN-20006.yaml b/contracts/OMN-20006.yaml new file mode 100644 index 0000000000..abfc6509f8 --- /dev/null +++ b/contracts/OMN-20006.yaml @@ -0,0 +1,55 @@ +--- +schema_version: "1.0.0" +ticket_id: "OMN-20006" +title: "The Usage page renders tokens in, tokens out and cost per model from one served exposure" +summary: "The data half of the Usage page. usage-by-model-day.v1 now keeps measured cost apart from cost that was estimated or not recorded: each call stores its usage_source, and each day-and-model row serves measured_cost_usd (NULL when no call was measured, never 0) and unmeasured_call_count beside cost_usd and call_count, which keep their meaning. The exposure is scoped to the reading tenant, the local onex delegate path writes the usage rows the page reads, a local store written before this change gains the new columns on connect, and the reasoned consumers opt-out is deleted because the omnidash Usage page is its reader. Tests: tests/test_omn19978_usage_by_model_day.py, tests/test_omn19978_usage_by_model_day_writer.py, tests/projection/test_omn20006_usage_by_model_day_served.py, tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py." +is_seam_ticket: false +interface_change: false +interfaces_touched: [] +requirements: + - id: "req-omn20006-usage-measured-cost-served" + statement: |- + usage-by-model-day.v1 serves, per tenant, UTC day and model, the tokens in and out of every call, the measured cost of the calls whose cost was measured, and the count of calls whose cost was not; the local onex delegate path writes those rows. + acceptance: + - id: "AC1" + statement: |- + AC1 (data half): one local onex delegate run writes one usage_by_model_day_calls row keyed by its correlation id with the terminal's tokens and its day-and-model row, two runs with equal tokens are two rows, a failing usage write never breaks the delegation, and a read through the projection read node returns the reading tenant's rows only -- falsifier: uv run pytest tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py tests/projection/test_omn20006_usage_by_model_day_served.py -q + - id: "AC3" + statement: |- + AC3 (data half): one measured and one estimated call for the same day and model give measured_cost_usd equal to the measured call's cost and unmeasured_call_count 1, on SQLite and in the Postgres writer's recount, and the usage source is stored and replays without double counting -- falsifier: uv run pytest tests/test_omn19978_usage_by_model_day.py tests/test_omn19978_usage_by_model_day_writer.py -q + - id: "AC4" + statement: |- + AC4 (data half): a day and model with no measured call serves measured_cost_usd as null, never 0, a measured call costing nothing serves a measured 0, and a call that does not say how its cost was obtained is unmeasured -- falsifier: uv run pytest tests/test_omn19978_usage_by_model_day.py -q -k "typed_null or measured_zero or no_usage_source" +evidence_requirements: + - kind: "tests" + description: "The usage tests pass at the head and fail at the merge base, where no usage source, measured cost, unmeasured count, tenant scope or local usage write exists." + command: "uv run pytest tests/test_omn19978_usage_by_model_day.py tests/test_omn19978_usage_by_model_day_writer.py tests/projection/test_omn20006_usage_by_model_day_served.py tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py -q" +emergency_bypass: + enabled: false + justification: "" + follow_up_ticket_id: "" +dod_evidence: + - id: "dod-omn20006-ac1-local-usage-rows-served-per-tenant" + description: "One local delegate run writes its usage call and rollup rows with the terminal's tokens, equal-token runs stay apart, a failed usage write is swallowed, and the served read carries the reading tenant's rows only." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py tests/projection/test_omn20006_usage_by_model_day_served.py -q" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC1"] + - id: "dod-omn20006-ac3-measured-cost-excludes-unmeasured" + description: "A measured and an estimated call give the measured call's cost as measured_cost_usd and one unmeasured call, on SQLite and in the Postgres recount statement; usage_source is stored and a replay changes nothing." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/test_omn19978_usage_by_model_day.py tests/test_omn19978_usage_by_model_day_writer.py -q" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC3"] + - id: "dod-omn20006-ac4-unmeasured-cost-is-null-not-zero" + description: "No measured call serves a null measured cost, a measured zero stays 0, and an unstated usage source is unmeasured." + source: "manual" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/test_omn19978_usage_by_model_day.py -q -k \"typed_null or measured_zero or no_usage_source\"" + cwd: "${OMNI_HOME}/omnimarket" + binds_ac: ["AC4"] diff --git a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py index 0db9a686cd..82459b4b25 100644 --- a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py +++ b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py @@ -214,6 +214,15 @@ HandlerProjectionLlmCost, ModelLlmCallCompletedEvent, ) +from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_projection_usage_by_model_day import ( + HandlerProjectionUsageByModelDay, +) +from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_usage_by_model_day_store import ( + apply_usage_call, +) +from omnimarket.nodes.node_projection_usage_by_model_day.models import ( + ModelUsageCallEvent, +) from omnimarket.pricing import ModelBaselineSavings, compute_baseline_savings from omnimarket.projection.protocol_database import DatabaseAdapter from omnimarket.projection.snapshot_publisher import ModelSnapshotDeltaMessage @@ -3344,6 +3353,32 @@ def _project_evidence( correlation_id, exc_info=True, ) + # OMN-20006: the same call, folded into the usage-by-model-day tables + # the Usage page reads, keyed by this run's correlation id and dated + # by the terminal's own emitted_at, so its UTC day is the delegation + # row's. Its own guard: a failed usage write must neither break the + # response nor take the call-metrics row down with it. + try: + usage_event = ModelUsageCallEvent( + call_id=str(correlation_id), + model_name=model_id, + tenant_id=tenant_id, + prompt_tokens=result.tokens_in, + completion_tokens=result.tokens_out, + estimated_cost_usd=float(cost_usd), + usage_source=EnumUsageSource.MEASURED, + timestamp=terminal.emitted_at, + ) + apply_usage_call( + HandlerProjectionUsageByModelDay().handle(usage_event), + self._evidence_db, + ) + except Exception: + logger.warning( + "Failed to project local usage-by-model-day for correlation_id=%s", + correlation_id, + exc_info=True, + ) except Exception: logger.warning( "Failed to project local delegation evidence for correlation_id=%s", diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/contract.yaml b/src/omnimarket/nodes/node_projection_usage_by_model_day/contract.yaml index 384f0d74b1..0e0170b111 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/contract.yaml +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/contract.yaml @@ -95,6 +95,11 @@ projection_api: - input_tokens - output_tokens - cost_usd + # OMN-20006: the cost of the calls whose cost was measured (NULL when none + # was), and how many calls were not measured. The Usage page shows these; + # cost_usd still sums every call. + - measured_cost_usd + - unmeasured_call_count - call_count - updated_at - projection_cursor @@ -104,18 +109,11 @@ projection_api: limit: 500 bus_backed: true key_columns: [tenant_id, usage_day, model_id] - # OMN-17199 -- the reader end. This exposure is `bus_backed: true` and no - # dashboard widget or typed backend reader renders it yet: OMN-19978 delivers - # the projection and its snapshot topic, not a rendering surface. The opt-out - # says so here instead of failing the exposure-reader gate on every - # omnibase_infra PR since the projection landed (omnimarket#3073). When a - # per-tenant usage surface reads it, delete both keys -- the gate fails on an - # opt-out left standing over a live reader. - consumers: none - consumers_reason: >- - Per-tenant model usage aggregates (tokens and cost per model per UTC day) - with no rendering surface built yet; the projection and its snapshot topic - ship first (OMN-19978) and a reader is a separate deliverable. Declared - here so the opt-out is visible rather than a silent gate failure. + # OMN-20006: a read is scoped to the reading tenant by the server, so the page + # never receives, or filters out, another tenant's rows. + tenant_column: tenant_id + # OMN-17199: the reader is the omnidash Usage page's usage-by-model-day + # component (OMN-20006), declared in omnidash's component registry, so the + # reasoned `consumers: none` opt-out is deleted rather than left standing. metadata: transport_type: kafka diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_projection_usage_by_model_day.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_projection_usage_by_model_day.py index 2a6695431e..89f9cbb5c8 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_projection_usage_by_model_day.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_projection_usage_by_model_day.py @@ -35,6 +35,7 @@ def handle(self, event: ModelUsageCallEvent) -> ModelUsageCallDelta: input_tokens=event.prompt_tokens, output_tokens=event.completion_tokens, cost_usd=Decimal(str(event.estimated_cost_usd)), + usage_source=event.usage_source, occurred_at=occurred_at, ) diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_store.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_store.py index 95bfea04b2..0b804bc157 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_store.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_store.py @@ -5,6 +5,7 @@ from datetime import UTC, datetime from decimal import Decimal +from omnimarket.enums.enum_usage_source import EnumUsageSource from omnimarket.nodes.node_projection_usage_by_model_day.models import ( ModelUsageCallDelta, ) @@ -33,11 +34,19 @@ def apply_usage_call(delta: ModelUsageCallDelta, db: DatabaseAdapter) -> bool: "input_tokens": delta.input_tokens, "output_tokens": delta.output_tokens, "cost_usd": delta.cost_usd, + "usage_source": delta.usage_source.value, "occurred_at": delta.occurred_at.isoformat(), "ingested_at": updated_at, }, ) calls = db.query(CALLS_TABLE, key) + # Only a measured cost is a measurement. A call stored before usage_source + # existed carries none, so it is counted as unmeasured, never as measured. + measured = [ + Decimal(str(row["cost_usd"])) + for row in calls + if row.get("usage_source") == EnumUsageSource.MEASURED.value + ] db.upsert( AGGREGATE_TABLE, "tenant_id,usage_day,model_id", @@ -48,6 +57,8 @@ def apply_usage_call(delta: ModelUsageCallDelta, db: DatabaseAdapter) -> bool: "cost_usd": sum( (Decimal(str(row["cost_usd"])) for row in calls), Decimal(0) ), + "measured_cost_usd": sum(measured, Decimal(0)) if measured else None, + "unmeasured_call_count": len(calls) - len(measured), "call_count": len(calls), "updated_at": updated_at, }, diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_writer.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_writer.py index 26cba8c3d9..16db6500ba 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_writer.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/handlers/handler_usage_by_model_day_writer.py @@ -28,19 +28,25 @@ _INSERT_CALL = """ INSERT INTO public.usage_by_model_day_calls ( call_id, tenant_id, usage_day, model_id, input_tokens, output_tokens, - cost_usd, occurred_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + cost_usd, occurred_at, usage_source + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) ON CONFLICT (call_id) DO NOTHING RETURNING call_id """ +# measured_cost_usd sums the measured calls only, and is NULL (not 0) when the key +# has none; unmeasured_call_count counts the rest. cost_usd and call_count keep +# their meaning: every call. _RECOUNT_AGGREGATE = """ INSERT INTO public.usage_by_model_day ( tenant_id, usage_day, model_id, input_tokens, output_tokens, - cost_usd, call_count, updated_at + cost_usd, measured_cost_usd, unmeasured_call_count, call_count, updated_at ) SELECT tenant_id, usage_day, model_id, SUM(input_tokens), SUM(output_tokens), - SUM(cost_usd), COUNT(*), clock_timestamp() + SUM(cost_usd), + SUM(cost_usd) FILTER (WHERE usage_source = 'measured'), + COUNT(*) FILTER (WHERE usage_source <> 'measured'), + COUNT(*), clock_timestamp() FROM public.usage_by_model_day_calls WHERE tenant_id = $1 AND usage_day = $2 AND model_id = $3 GROUP BY tenant_id, usage_day, model_id @@ -48,11 +54,14 @@ input_tokens = EXCLUDED.input_tokens, output_tokens = EXCLUDED.output_tokens, cost_usd = EXCLUDED.cost_usd, + measured_cost_usd = EXCLUDED.measured_cost_usd, + unmeasured_call_count = EXCLUDED.unmeasured_call_count, call_count = EXCLUDED.call_count, updated_at = EXCLUDED.updated_at, projection_cursor = EXCLUDED.projection_cursor RETURNING tenant_id, usage_day, model_id, input_tokens, output_tokens, - cost_usd, call_count, updated_at, projection_cursor + cost_usd, measured_cost_usd, unmeasured_call_count, call_count, + updated_at, projection_cursor """ @@ -168,6 +177,7 @@ async def _project_call( delta.output_tokens, delta.cost_usd, delta.occurred_at, + delta.usage_source.value, ) if not inserted: return None diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/migrations/0002_usage_by_model_day_measured_cost.sql b/src/omnimarket/nodes/node_projection_usage_by_model_day/migrations/0002_usage_by_model_day_measured_cost.sql new file mode 100644 index 0000000000..3ca0b882c2 --- /dev/null +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/migrations/0002_usage_by_model_day_measured_cost.sql @@ -0,0 +1,15 @@ +-- OMN-20006: separate measured cost from cost that was estimated or not recorded. +-- Owner: omnimarket.nodes.node_projection_usage_by_model_day +-- Target database: omnidash_analytics; physical schema: public. +-- +-- Additive only. A call stored before this migration reads usage_source +-- 'unknown'; an aggregate row reads measured_cost_usd NULL and +-- unmeasured_call_count 0 until the next call for its key recounts it. Nothing +-- is rewritten, so old code keeps working and a rollback needs no data change. +-- Re-runnable: every statement is IF NOT EXISTS. + +ALTER TABLE public.usage_by_model_day_calls ADD COLUMN IF NOT EXISTS usage_source TEXT NOT NULL DEFAULT 'unknown'; + +-- NULL means no call for the key had a measured cost; it is never written as 0. +ALTER TABLE public.usage_by_model_day ADD COLUMN IF NOT EXISTS measured_cost_usd NUMERIC(18,8) CHECK (measured_cost_usd >= 0); +ALTER TABLE public.usage_by_model_day ADD COLUMN IF NOT EXISTS unmeasured_call_count INTEGER NOT NULL DEFAULT 0 CHECK (unmeasured_call_count >= 0); diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_delta.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_delta.py index e8b0867697..8656cc4700 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_delta.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_delta.py @@ -6,6 +6,8 @@ from pydantic import AwareDatetime, BaseModel, ConfigDict, Field +from omnimarket.enums.enum_usage_source import EnumUsageSource + class ModelUsageCallDelta(BaseModel): model_config = ConfigDict(frozen=True, extra="forbid") @@ -17,6 +19,7 @@ class ModelUsageCallDelta(BaseModel): input_tokens: int = Field(ge=0) output_tokens: int = Field(ge=0) cost_usd: Decimal = Field(ge=0) + usage_source: EnumUsageSource occurred_at: AwareDatetime diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py index 6349e4698c..5b7df15b21 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py @@ -6,6 +6,8 @@ from pydantic import AliasChoices, BaseModel, ConfigDict, Field, field_validator +from omnimarket.enums.enum_usage_source import EnumUsageSource + class ModelUsageCallEvent(BaseModel): model_config = ConfigDict(frozen=True, extra="ignore", populate_by_name=True) @@ -32,6 +34,9 @@ class ModelUsageCallEvent(BaseModel): allow_inf_nan=False, validation_alias=AliasChoices("estimated_cost_usd", "cost_usd"), ) + # How the call's cost was obtained. A call that does not say is UNKNOWN, never + # assumed measured: only a measured cost may reach measured_cost_usd. + usage_source: EnumUsageSource = EnumUsageSource.UNKNOWN session_id: str | None = None timestamp: datetime = Field( validation_alias=AliasChoices("timestamp", "emitted_at", "created_at") diff --git a/src/omnimarket/projection/sqlite_database.py b/src/omnimarket/projection/sqlite_database.py index 63c9ca2214..3a2943b592 100644 --- a/src/omnimarket/projection/sqlite_database.py +++ b/src/omnimarket/projection/sqlite_database.py @@ -114,7 +114,8 @@ output_tokens INTEGER NOT NULL, cost_usd REAL NOT NULL, occurred_at TEXT NOT NULL, - ingested_at TEXT NOT NULL + ingested_at TEXT NOT NULL, + usage_source TEXT NOT NULL DEFAULT 'unknown' ) """ @@ -126,12 +127,64 @@ input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, cost_usd REAL NOT NULL, + measured_cost_usd REAL, + unmeasured_call_count INTEGER NOT NULL DEFAULT 0, call_count INTEGER NOT NULL, updated_at TEXT NOT NULL, + projection_cursor INTEGER, PRIMARY KEY (tenant_id, usage_day, model_id) ) """ +# OMN-20006: columns the usage-by-model-day exposure serves that a store written +# before them lacks. The read node refuses an exposure whose declared column the +# table lacks (projection_column_missing), so they are added on connect, with the +# same defaults as migration 0002: an old call reads 'unknown', an old aggregate +# NULL measured cost and 0 unmeasured calls until its key is recounted. +_USAGE_BY_MODEL_DAY_ADDED_COLUMNS: tuple[tuple[str, str, str], ...] = ( + ("usage_by_model_day_calls", "usage_source", "TEXT NOT NULL DEFAULT 'unknown'"), + ("usage_by_model_day", "measured_cost_usd", "REAL"), + ("usage_by_model_day", "unmeasured_call_count", "INTEGER NOT NULL DEFAULT 0"), + ("usage_by_model_day", "projection_cursor", "INTEGER"), +) + +# The exposure's cursor. On Postgres it is a BIGSERIAL that every recount +# re-stamps from its sequence, so a row read again after a recount sorts after +# the rows read before it. SQLite gets the same from a one-row sequence table +# and two triggers: each insert and each recount (which always sets updated_at) +# takes the next value. SQLite does not fire triggers recursively by default, so +# the trigger's own UPDATE does not re-enter. A row stored before the cursor +# existed has none until its key is next recounted; no local path wrote usage +# rows before this change, so a local store has none in practice. +_USAGE_BY_MODEL_DAY_CURSOR_SEQ_DDL = """ +CREATE TABLE IF NOT EXISTS usage_by_model_day_projection_cursor_seq ( + id INTEGER PRIMARY KEY CHECK (id = 1), + last_value INTEGER NOT NULL +) +""" +_USAGE_BY_MODEL_DAY_CURSOR_SEQ_SEED = ( + "INSERT OR IGNORE INTO usage_by_model_day_projection_cursor_seq " + "(id, last_value) VALUES (1, 0)" +) +_USAGE_BY_MODEL_DAY_CURSOR_STAMP = """ + UPDATE usage_by_model_day_projection_cursor_seq SET last_value = last_value + 1; + UPDATE usage_by_model_day + SET projection_cursor = ( + SELECT last_value FROM usage_by_model_day_projection_cursor_seq + ) + WHERE rowid = NEW.rowid; +""" +_USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS = ( + "CREATE TRIGGER IF NOT EXISTS usage_by_model_day_cursor_on_insert " + "AFTER INSERT ON usage_by_model_day BEGIN" + + _USAGE_BY_MODEL_DAY_CURSOR_STAMP + + "END", + "CREATE TRIGGER IF NOT EXISTS usage_by_model_day_cursor_on_recount " + "AFTER UPDATE OF updated_at ON usage_by_model_day BEGIN" + + _USAGE_BY_MODEL_DAY_CURSOR_STAMP + + "END", +) + _METERING_SUMMARY_DDL = """ CREATE TABLE IF NOT EXISTS metering_summary ( tenant_id TEXT NOT NULL, @@ -293,6 +346,7 @@ def _connect(self) -> sqlite3.Connection: conn.execute(_DELEGATE_SKILL_CLAIMS_DDL) conn.execute(_USAGE_BY_MODEL_DAY_CALLS_DDL) conn.execute(_USAGE_BY_MODEL_DAY_DDL) + self._reconcile_usage_by_model_day(conn) conn.execute(_METERING_SUMMARY_DDL) conn.execute(_METERING_SUMMARY_INDEX_DDL) conn.execute(_LLM_CALL_METRICS_DDL) @@ -302,6 +356,40 @@ def _connect(self) -> sqlite3.Connection: conn.commit() return conn + @classmethod + def _usage_by_model_day_reconciled(cls, conn: sqlite3.Connection) -> bool: + for table, column, _ in _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: + if column not in cls._existing_columns(conn, table): + return False + triggers = conn.execute( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'trigger' AND name IN " + "('usage_by_model_day_cursor_on_insert', " + "'usage_by_model_day_cursor_on_recount')" + ).fetchone()[0] + return bool(triggers == len(_USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS)) + + @classmethod + def _reconcile_usage_by_model_day(cls, conn: sqlite3.Connection) -> None: + if cls._usage_by_model_day_reconciled(conn): + return + # Two first opens of one store can race: the check and the ALTERs run in + # one write transaction, so the second never adds a column twice. + conn.execute("BEGIN IMMEDIATE") + try: + for table, column, declaration in _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: + if column not in cls._existing_columns(conn, table): + conn.execute( + f"ALTER TABLE {table} ADD COLUMN {column} {declaration}" + ) + conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_DDL) + conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_SEED) + for trigger in _USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS: + conn.execute(trigger) + conn.commit() + except BaseException: + conn.rollback() + raise + @classmethod def _reconcile_legacy_llm_call_metrics(cls, conn: sqlite3.Connection) -> None: columns = cls._existing_columns(conn, "llm_call_metrics") diff --git a/tests/projection/test_omn20006_usage_by_model_day_served.py b/tests/projection/test_omn20006_usage_by_model_day_served.py new file mode 100644 index 0000000000..909e009ac5 --- /dev/null +++ b/tests/projection/test_omn20006_usage_by_model_day_served.py @@ -0,0 +1,133 @@ +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""OMN-20006: the Usage page's exposure serves measured cost per tenant. + +Calls for two tenants are folded and stored in a local SQLite store the way the +local writer does, then read back through the projection read node exactly as +``onex dashboard`` serves them, with the exposures built from the node contracts +(no hand-built topic map), so the contract declaration is what is under test. + +Failure modes each test is written against: + +* the exposure is not tenant-scoped: the reading tenant gets another tenant's rows, + and the page would have to filter in the browser; +* the contract leaves ``measured_cost_usd`` or ``unmeasured_call_count`` out of the + served row, so the page cannot tell measured cost from an estimate; +* a key with no measured call is served as ``0`` instead of a typed null; +* an estimated call's cost leaks into the served measured cost. +""" + +from __future__ import annotations + +from decimal import Decimal +from functools import cache +from pathlib import Path +from typing import Any + +import pytest +from fastapi.testclient import TestClient + +from omnimarket.nodes.node_local_dashboard_serve_effect.handlers.handler_local_dashboard_serve import ( + create_dashboard_app, +) +from omnimarket.nodes.node_projection_read_effect.handlers.handler_projection_read import ( + HandlerProjectionRead, +) +from omnimarket.nodes.node_projection_read_effect.ports.sqlite_row_source import ( + SqliteTableRowSource, +) +from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_projection_usage_by_model_day import ( + HandlerProjectionUsageByModelDay, +) +from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_usage_by_model_day_store import ( + apply_usage_call, +) +from omnimarket.nodes.node_projection_usage_by_model_day.models import ( + ModelUsageCallEvent, +) +from omnimarket.projection.discovery import build_projection_topic_map +from omnimarket.projection.models import ProjectionTableConfig +from omnimarket.projection.sqlite_database import SqliteDatabaseAdapter + +pytestmark = pytest.mark.unit + +USAGE = "onex.snapshot.projection.usage-by-model-day.v1" +TENANT = "820272f9-4aaf-5add-a2df-0af942852ab2" +OTHER_TENANT = "11111111-2222-4333-8444-555555555555" + +# (tenant, call_id, model, timestamp, tokens in, tokens out, cost, usage_source) +CALLS: tuple[tuple[str, str, str, str, int, int, float, str], ...] = ( + (TENANT, "t1-a", "local-a", "2026-09-27T09:00:00Z", 100, 10, 0.25, "measured"), + (TENANT, "t1-b", "local-a", "2026-09-27T10:00:00Z", 50, 5, 9.0, "estimated"), + (TENANT, "t1-c", "local-b", "2026-09-28T10:00:00Z", 30, 3, 4.0, "unknown"), + (OTHER_TENANT, "t2-a", "local-a", "2026-09-27T09:00:00Z", 7, 1, 2.0, "measured"), +) + + +def _store(tmp_path: Path) -> Path: + db_path = tmp_path / "delegation.sqlite" + adapter = SqliteDatabaseAdapter(db_path) + fold = HandlerProjectionUsageByModelDay() + for tenant, call_id, model, stamp, tin, tout, cost, source in CALLS: + event = ModelUsageCallEvent.model_validate( + { + "call_id": call_id, + "model_name": model, + "timestamp": stamp, + "prompt_tokens": tin, + "completion_tokens": tout, + "estimated_cost_usd": cost, + "usage_source": source, + "tenant_id": tenant, + } + ) + apply_usage_call(fold.handle(event), adapter) + return db_path + + +@cache +def _topics() -> dict[str, ProjectionTableConfig]: + """The exposures exactly as the node contracts declare them.""" + return build_projection_topic_map() + + +def _read(db_path: Path, tenant: str) -> list[dict[str, Any]]: + topics = _topics() + handler = HandlerProjectionRead( + topic_map=topics, row_source=SqliteTableRowSource(db_path) + ) + client = TestClient( + create_dashboard_app(handler=handler, topic_map=topics, tenant=tenant) + ) + response = client.get(f"/projection/{USAGE}") + assert response.status_code == 200, response.json() + rows: list[dict[str, Any]] = response.json()["rows"] + return rows + + +def test_a_tenant_reads_only_its_own_usage_rows(tmp_path: Path) -> None: + db_path = _store(tmp_path) + mine = _read(db_path, TENANT) + theirs = _read(db_path, OTHER_TENANT) + assert {r["tenant_id"] for r in mine} == {TENANT} + assert {(r["usage_day"], r["model_id"]) for r in mine} == { + ("2026-09-27", "local-a"), + ("2026-09-28", "local-b"), + } + assert [(r["tenant_id"], r["model_id"]) for r in theirs] == [ + (OTHER_TENANT, "local-a") + ] + + +def test_the_served_row_carries_measured_cost_and_the_unmeasured_count( + tmp_path: Path, +) -> None: + rows = {(r["usage_day"], r["model_id"]): r for r in _read(_store(tmp_path), TENANT)} + mixed = rows[("2026-09-27", "local-a")] + assert Decimal(str(mixed["measured_cost_usd"])) == Decimal("0.25") + assert mixed["unmeasured_call_count"] == 1 + assert Decimal(str(mixed["cost_usd"])) == Decimal("9.25") + assert mixed["call_count"] == 2 + unmeasured_only = rows[("2026-09-28", "local-b")] + assert unmeasured_only["measured_cost_usd"] is None + assert unmeasured_only["unmeasured_call_count"] == 1 diff --git a/tests/test_omn19968_writer_store_parity.py b/tests/test_omn19968_writer_store_parity.py index ef917f31ee..0a9738dd62 100644 --- a/tests/test_omn19968_writer_store_parity.py +++ b/tests/test_omn19968_writer_store_parity.py @@ -323,6 +323,10 @@ def test_both_stores_take_the_attested_insert_only_branch(tmp_path: Path) -> Non _NODES / "node_projection_usage_by_model_day/migrations/0000_create_usage_by_model_day.sql" ) +_USAGE_MEASURED_MIGRATION = ( + _NODES + / "node_projection_usage_by_model_day/migrations/0002_usage_by_model_day_measured_cost.sql" +) _METERING_MIGRATION = ( _NODES / "node_projection_metering_summary/migrations/0000_create_metering_summary.sql" @@ -332,10 +336,47 @@ def test_both_stores_take_the_attested_insert_only_branch(tmp_path: Path) -> Non _METERING_NOW = datetime(2026, 9, 28, 12, tzinfo=__import__("datetime").UTC) +# OMN-20006: every usage source on both stores, so the measured-cost and +# unmeasured-count columns are compared, not only the token and cost sums. The +# 2026-09-28 qwen3-coder key (c01-c03) has no measured call, so its measured +# cost is NULL on both stores; ``_normalize`` drops None values, so that key is +# compared explicitly in the test below. +_USAGE_SOURCES = { + "c01": "estimated", + "c02": "unknown", + "c03": "unknown", + "c04": "measured", + "c05": "estimated", + "c06": "measured", + "c07": "unknown", + "c08": "measured", + "c09": "measured", + "c10": "estimated", +} + + def _write_usage(adapter: Any) -> None: fold = HandlerProjectionUsageByModelDay() for call in CALLS: - apply_usage_call(fold.handle(_event(call)), adapter) + event = _event(call).model_copy( + update={"usage_source": _USAGE_SOURCES[call[0]]} + ) + apply_usage_call(fold.handle(event), adapter) + + +def _measured_by_key( + rows: list[dict[str, object]], +) -> dict[tuple[str, str], tuple[object, int]]: + """(day, model) -> (measured cost as Decimal or None, unmeasured count).""" + return { + (str(r["usage_day"])[:10], str(r["model_id"])): ( + None + if r["measured_cost_usd"] is None + else Decimal(str(r["measured_cost_usd"])).normalize(), + int(str(r["unmeasured_call_count"])), + ) + for r in rows + } def _metering_request() -> ModelMeteringSummaryFoldRequest: @@ -408,12 +449,21 @@ async def test_store_neutral_rows_equal_on_sqlite_and_postgres( async with _provisioned(pg) as (admin, schema): # The migration names ``public.``; keep the proof inside the throwaway schema. - await admin.execute( - migration.read_text(encoding="utf-8").replace("public.", f"{schema}.") + migrations = ( + (migration, _USAGE_MEASURED_MIGRATION) + if migration == _USAGE_MIGRATION + else (migration,) ) + for each in migrations: + await admin.execute( + each.read_text(encoding="utf-8").replace("public.", f"{schema}.") + ) postgres = PostgresSyncProjectionAdapter(_dsn(pg, schema)) writer(postgres) pg_rows = {t: _normalize_store(postgres.query(t)) for t in tables} + pg_usage = ( + postgres.query("usage_by_model_day") if case == "usage_by_model_day" else [] + ) for table in tables: assert sqlite_rows[table], f"SQLite path wrote no {table} rows" @@ -452,6 +502,11 @@ def _by_common( assert set().union(*sqlite_rows[table]) <= ( set().union(*pg_rows[table]) | _GENERATED | _ALSO_GENERATED ) + if case == "usage_by_model_day": + sqlite_measured = _measured_by_key(sqlite.query("usage_by_model_day")) + assert sqlite_measured == _measured_by_key(pg_usage) + # The key with no measured call is NULL on both stores, never 0. + assert sqlite_measured[("2026-09-28", "qwen3-coder")] == (None, 3) offending = [s for s in sqlite.statements if _POSTGRES_ONLY_SQL.search(s)] assert offending == [] diff --git a/tests/test_omn19978_usage_by_model_day.py b/tests/test_omn19978_usage_by_model_day.py index b97a2babf8..98a247c963 100644 --- a/tests/test_omn19978_usage_by_model_day.py +++ b/tests/test_omn19978_usage_by_model_day.py @@ -9,6 +9,7 @@ from __future__ import annotations +import sqlite3 from decimal import Decimal from pathlib import Path @@ -16,6 +17,7 @@ import yaml from pydantic import ValidationError +from omnimarket.enums.enum_usage_source import EnumUsageSource from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_projection_usage_by_model_day import ( HandlerProjectionUsageByModelDay, ) @@ -144,6 +146,193 @@ def test_fold_is_pure_and_replay_deterministic() -> None: assert fold.handle(event) == fold.handle(event) +def _sourced( + call_id: str, cost: float, source: str | None, *, tenant: str = "omninode" +) -> ModelUsageCallEvent: + payload: dict[str, object] = { + "call_id": call_id, + "model_name": "m", + "timestamp": "2026-09-28T00:00:00Z", + "prompt_tokens": 10, + "completion_tokens": 2, + "estimated_cost_usd": cost, + "tenant_id": tenant, + } + if source is not None: + payload["usage_source"] = source + return ModelUsageCallEvent.model_validate(payload) + + +def _aggregate_after( + tmp_path: Path, *events: ModelUsageCallEvent +) -> tuple[SqliteDatabaseAdapter, dict[str, object]]: + db = SqliteDatabaseAdapter(tmp_path / "usage.sqlite") + fold = HandlerProjectionUsageByModelDay() + for event in events: + apply_usage_call(fold.handle(event), db) + (row,) = db.query(AGGREGATE_TABLE) + return db, row + + +def test_usage_source_is_stored_and_estimated_cost_is_not_measured( + tmp_path: Path, +) -> None: + """AC3: one measured and one estimated call for the same day and model. + + The measured cost is the measured call's alone, the estimated call is counted + as unmeasured, and ``cost_usd``/``call_count`` keep their meaning (every call). + """ + db, row = _aggregate_after( + tmp_path, + _sourced("measured-1", 0.25, "measured"), + _sourced("estimated-1", 9.0, "estimated"), + ) + calls = {r["call_id"]: r["usage_source"] for r in db.query(CALLS_TABLE)} + assert calls == {"measured-1": "measured", "estimated-1": "estimated"} + assert Decimal(str(row["measured_cost_usd"])) == Decimal("0.25") + assert int(str(row["unmeasured_call_count"])) == 1 + assert Decimal(str(row["cost_usd"])) == Decimal("9.25") + assert int(str(row["call_count"])) == 2 + assert (int(str(row["input_tokens"])), int(str(row["output_tokens"]))) == (20, 4) + + +def test_unmeasured_only_aggregate_has_typed_null_cost(tmp_path: Path) -> None: + """AC4: no measured call means no measured cost -- NULL, never 0.""" + _, row = _aggregate_after(tmp_path, _sourced("unknown-1", 4.0, "unknown")) + assert row["measured_cost_usd"] is None + assert int(str(row["unmeasured_call_count"])) == 1 + + +def test_a_measured_call_costing_nothing_is_a_measured_zero_not_null( + tmp_path: Path, +) -> None: + _, row = _aggregate_after(tmp_path, _sourced("free-1", 0.0, "measured")) + assert row["measured_cost_usd"] is not None + assert Decimal(str(row["measured_cost_usd"])) == Decimal(0) + assert int(str(row["unmeasured_call_count"])) == 0 + + +def test_a_call_with_no_usage_source_is_unmeasured_not_assumed_measured( + tmp_path: Path, +) -> None: + """An event that never said how its cost was obtained is not a measurement.""" + event = _sourced("silent-1", 3.0, None) + assert event.usage_source == EnumUsageSource.UNKNOWN + _, row = _aggregate_after(tmp_path, event) + assert row["measured_cost_usd"] is None + assert int(str(row["unmeasured_call_count"])) == 1 + + +def test_the_legacy_api_source_reads_as_measured() -> None: + """llm_call_metrics stores the legacy alias API for a measured call.""" + assert _sourced("api-1", 1.0, "API").usage_source == EnumUsageSource.MEASURED + + +def test_the_fold_carries_the_usage_source_into_the_delta() -> None: + delta = HandlerProjectionUsageByModelDay().handle(_sourced("e-1", 1.0, "estimated")) + assert delta.usage_source == EnumUsageSource.ESTIMATED + + +def test_replaying_a_mixed_key_changes_nothing(tmp_path: Path) -> None: + events = ( + _sourced("measured-1", 0.25, "measured"), + _sourced("estimated-1", 9.0, "estimated"), + ) + db, before = _aggregate_after(tmp_path, *events) + fold = HandlerProjectionUsageByModelDay() + for event in events: + assert apply_usage_call(fold.handle(event), db) is False + assert db.query(AGGREGATE_TABLE) == [before] + + +def test_an_old_local_store_gains_the_new_columns_on_connect(tmp_path: Path) -> None: + """A store written before this change keeps working: old rows read unknown/NULL/0.""" + path = tmp_path / "old.sqlite" + conn = sqlite3.connect(path) + conn.execute( + "CREATE TABLE usage_by_model_day_calls (call_id TEXT PRIMARY KEY, " + "tenant_id TEXT NOT NULL, usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, occurred_at TEXT NOT NULL, ingested_at TEXT NOT NULL)" + ) + conn.execute( + "CREATE TABLE usage_by_model_day (tenant_id TEXT NOT NULL, " + "usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, call_count INTEGER NOT NULL, " + "updated_at TEXT NOT NULL, PRIMARY KEY (tenant_id, usage_day, model_id))" + ) + conn.execute( + "INSERT INTO usage_by_model_day_calls VALUES " + "('old-1', 'omninode', '2026-09-27', 'm', 1, 1, 0.5, 'x', 'y')" + ) + conn.execute( + "INSERT INTO usage_by_model_day VALUES " + "('omninode', '2026-09-27', 'm', 1, 1, 0.5, 1, 'y')" + ) + conn.commit() + conn.close() + + db = SqliteDatabaseAdapter(path) + (old,) = db.query(AGGREGATE_TABLE) + assert old["measured_cost_usd"] is None + assert int(str(old["unmeasured_call_count"])) == 0 + assert db.query(CALLS_TABLE)[0]["usage_source"] == "unknown" + # The next call for that key recounts it with the old call as unmeasured. + apply_usage_call( + HandlerProjectionUsageByModelDay().handle( + ModelUsageCallEvent.model_validate( + { + "call_id": "new-1", + "model_name": "m", + "timestamp": "2026-09-27T10:00:00Z", + "estimated_cost_usd": 0.1, + "usage_source": "measured", + "tenant_id": "omninode", + } + ) + ), + db, + ) + (row,) = db.query(AGGREGATE_TABLE) + assert Decimal(str(row["measured_cost_usd"])) == Decimal("0.1") + assert int(str(row["unmeasured_call_count"])) == 1 + assert int(str(row["call_count"])) == 2 + + +def test_each_insert_and_recount_takes_the_next_cursor(tmp_path: Path) -> None: + """The served exposure walks by projection_cursor, so a local store must + stamp one, and re-stamp a recounted row the way the Postgres BIGSERIAL does.""" + db = SqliteDatabaseAdapter(tmp_path / "usage.sqlite") + fold = HandlerProjectionUsageByModelDay() + apply_usage_call(fold.handle(_sourced("a", 1.0, "measured")), db) + other = _sourced("b", 1.0, "measured").model_copy(update={"model_name": "n"}) + apply_usage_call(fold.handle(other), db) + cursors = {r["model_id"]: r["projection_cursor"] for r in db.query(AGGREGATE_TABLE)} + assert cursors == {"m": 1, "n": 2} + apply_usage_call(fold.handle(_sourced("c", 1.0, "measured")), db) + cursors = {r["model_id"]: r["projection_cursor"] for r in db.query(AGGREGATE_TABLE)} + assert cursors == {"m": 3, "n": 2} + + +def test_the_measured_cost_migration_is_additive() -> None: + """Existing rows read unknown / NULL / 0; nothing is dropped or rewritten.""" + sql = ( + NODE_DIR / "migrations" / "0002_usage_by_model_day_measured_cost.sql" + ).read_text() + for statement in ( + "ALTER TABLE public.usage_by_model_day_calls ADD COLUMN IF NOT EXISTS " + "usage_source TEXT NOT NULL DEFAULT 'unknown'", + "ALTER TABLE public.usage_by_model_day ADD COLUMN IF NOT EXISTS " + "measured_cost_usd NUMERIC(18,8)", + "ALTER TABLE public.usage_by_model_day ADD COLUMN IF NOT EXISTS " + "unmeasured_call_count INTEGER NOT NULL DEFAULT 0", + ): + assert statement in sql + for forbidden in ("DROP ", "UPDATE ", "DELETE ", "TRUNCATE "): + assert forbidden not in sql.upper() + + def test_a_missing_tenant_falls_to_the_house_tenant_not_an_empty_key() -> None: event = ModelUsageCallEvent.model_validate( { @@ -180,22 +369,36 @@ def test_exposure_is_bus_backed_at_the_model_day_grain() -> None: assert exposure.bus_backed is True assert exposure.cursor_column == "projection_cursor" assert exposure.key_columns == ("tenant_id", "usage_day", "model_id") - for column in ("input_tokens", "output_tokens", "cost_usd", "call_count"): + for column in ( + "input_tokens", + "output_tokens", + "cost_usd", + "measured_cost_usd", + "unmeasured_call_count", + "call_count", + ): assert column in exposure.columns -def test_exposure_declares_no_reader_until_a_surface_reads_it() -> None: - # OMN-17199: a bus_backed exposure with no reader fails the omnibase_infra - # exposure-reader-coverage gate on every PR. No dashboard or backend reader - # renders this projection yet, so the opt-out is declared with its reason; - # delete both keys when a reader lands. +def test_exposure_is_tenant_scoped() -> None: + """AC-T: a read for one tenant never carries another tenant's rows.""" + contract = _contract() + (exposure,) = load_projection_exposures_from_contract( + contract, str(contract["name"]), CONTRACT_PATH + ) + assert exposure.tenant_column == "tenant_id" + + +def test_the_reader_opt_out_is_gone_now_that_the_usage_page_reads_it() -> None: + # OMN-17199: `consumers: none` standing over a live reader fails the + # omnibase_infra exposure-reader-coverage gate (stale_opt_out), and any other + # `consumers` value fails closed. The Usage page (omnidash, OMN-20006) is the + # declared reader, so both keys are deleted, not rewritten. api = _contract()["projection_api"] assert isinstance(api, dict) assert api["bus_backed"] is True - assert api["consumers"] == "none" - reason = api["consumers_reason"] - assert isinstance(reason, str) - assert "no rendering surface" in reason + assert "consumers" not in api + assert "consumers_reason" not in api def test_runtime_dispatch_resolves_only_the_writer() -> None: diff --git a/tests/test_omn19978_usage_by_model_day_real_postgres.py b/tests/test_omn19978_usage_by_model_day_real_postgres.py index 19acb14e4e..23ffef9e45 100644 --- a/tests/test_omn19978_usage_by_model_day_real_postgres.py +++ b/tests/test_omn19978_usage_by_model_day_real_postgres.py @@ -25,12 +25,20 @@ _INSERT_CALL, _RECOUNT_AGGREGATE, ) +from omnimarket.nodes.node_projection_usage_by_model_day.models import ( + ModelUsageCallDelta, + ModelUsageCallEvent, +) from tests.test_omn19978_usage_by_model_day import CALLS, _event -_MIGRATION = ( +_MIGRATIONS = tuple( Path(__file__).resolve().parents[1] - / "src/omnimarket/nodes/node_projection_usage_by_model_day" - / "migrations/0000_create_usage_by_model_day.sql" + / "src/omnimarket/nodes/node_projection_usage_by_model_day/migrations" + / name + for name in ( + "0000_create_usage_by_model_day.sql", + "0002_usage_by_model_day_measured_cost.sql", + ) ) _SCHEMA = "omn19978_usage_by_model_day_test" @@ -71,6 +79,7 @@ async def _apply_all(conn: asyncpg.Connection) -> None: delta.output_tokens, delta.cost_usd, delta.occurred_at, + delta.usage_source.value, ) if inserted: await conn.fetch( @@ -86,9 +95,10 @@ async def test_real_postgres_ten_calls_give_four_rows_and_a_replay_changes_nothi try: await conn.execute(f"DROP SCHEMA IF EXISTS {_SCHEMA} CASCADE") await conn.execute(f"CREATE SCHEMA {_SCHEMA}") - await conn.execute( - _MIGRATION.read_text(encoding="utf-8").replace("public.", f"{_SCHEMA}.") - ) + for migration in _MIGRATIONS: + await conn.execute( + migration.read_text(encoding="utf-8").replace("public.", f"{_SCHEMA}.") + ) await _apply_all(conn) rows = await conn.fetch( f"SELECT * FROM {_SCHEMA}.usage_by_model_day ORDER BY usage_day, model_id" @@ -108,3 +118,88 @@ async def test_real_postgres_ten_calls_give_four_rows_and_a_replay_changes_nothi finally: await conn.execute(f"DROP SCHEMA IF EXISTS {_SCHEMA} CASCADE") await conn.close() + + +@pytest.mark.integration +async def test_real_postgres_measured_cost_excludes_unmeasured_calls() -> None: + """OMN-20006 AC3/AC4 on real Postgres: the recount's FILTER clauses and the + typed NULL for a key with no measured call, plus 0002 over existing rows.""" + conn = await _connect_or_skip() + schema = f"{_SCHEMA}_measured" + try: + await conn.execute(f"DROP SCHEMA IF EXISTS {schema} CASCADE") + await conn.execute(f"CREATE SCHEMA {schema}") + base, measured = (m.read_text(encoding="utf-8") for m in _MIGRATIONS) + await conn.execute(base.replace("public.", f"{schema}.")) + # A row written before 0002 exists: the migration must keep it, as + # unknown / NULL / 0, and be re-runnable. + await conn.execute( + f"INSERT INTO {schema}.usage_by_model_day_calls (call_id, tenant_id, " + "usage_day, model_id, input_tokens, output_tokens, cost_usd, occurred_at) " + "VALUES ('old', 't', '2026-09-27', 'm', 1, 1, 0.5, '2026-09-27T00:00Z')" + ) + for _ in range(2): + await conn.execute(measured.replace("public.", f"{schema}.")) + old = await conn.fetchrow( + f"SELECT usage_source FROM {schema}.usage_by_model_day_calls" + ) + assert old is not None + assert old["usage_source"] == "unknown" + + def _event_of( + call_id: str, day: str, cost: float, source: str + ) -> ModelUsageCallDelta: + return HandlerProjectionUsageByModelDay().handle( + ModelUsageCallEvent.model_validate( + { + "call_id": call_id, + "model_name": "m", + "timestamp": f"{day}T10:00:00Z", + "prompt_tokens": 10, + "completion_tokens": 2, + "estimated_cost_usd": cost, + "usage_source": source, + "tenant_id": "t", + } + ) + ) + + for call_id, day, cost, source in ( + ("m1", "2026-09-28", 0.25, "measured"), + ("e1", "2026-09-28", 9.0, "estimated"), + ("u1", "2026-09-29", 4.0, "unknown"), + ): + delta = _event_of(call_id, day, cost, source) + usage_day = date.fromisoformat(delta.usage_day) + await conn.fetch( + _INSERT_CALL.replace("public.", f"{schema}."), + delta.call_id, + delta.tenant_id, + usage_day, + delta.model_id, + delta.input_tokens, + delta.output_tokens, + delta.cost_usd, + delta.occurred_at, + delta.usage_source.value, + ) + await conn.fetch( + _RECOUNT_AGGREGATE.replace("public.", f"{schema}."), + delta.tenant_id, + usage_day, + delta.model_id, + ) + rows = { + str(r["usage_day"]): r + for r in await conn.fetch( + f"SELECT * FROM {schema}.usage_by_model_day ORDER BY usage_day" + ) + } + assert rows["2026-09-28"]["measured_cost_usd"] == Decimal("0.25") + assert rows["2026-09-28"]["unmeasured_call_count"] == 1 + assert rows["2026-09-28"]["cost_usd"] == Decimal("9.25") + assert rows["2026-09-29"]["measured_cost_usd"] is None + assert rows["2026-09-29"]["unmeasured_call_count"] == 1 + finally: + await conn.execute(f"DROP SCHEMA IF EXISTS {schema} CASCADE") + await conn.close() diff --git a/tests/test_omn19978_usage_by_model_day_writer.py b/tests/test_omn19978_usage_by_model_day_writer.py index 8ecfebd5cb..c6e25736cc 100644 --- a/tests/test_omn19978_usage_by_model_day_writer.py +++ b/tests/test_omn19978_usage_by_model_day_writer.py @@ -124,10 +124,34 @@ async def published(*args, **kwargs): 3, Decimal("0.001"), datetime(2026, 9, 29, 3, 30, tzinfo=UTC), + # OMN-20006: a payload that never says how its cost was obtained is stored + # as unknown, so the recount keeps it out of measured_cost_usd. + "unknown", ) writer.publish_snapshot_delta.assert_awaited_once() +async def test_the_usage_source_is_bound_on_insert(writer_boundary) -> None: + writer, _, conn = writer_boundary + conn.fetch.return_value = [] + payload = _payload() + payload["usage_source"] = "API" + await writer.project_event(writer.topics[0], payload, _meta(writer)) + assert conn.fetch.await_args_list[0].args[-1] == "measured" + + +def test_the_recount_sums_only_measured_cost_and_counts_the_rest() -> None: + from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_usage_by_model_day_writer import ( + _RECOUNT_AGGREGATE, + ) + + sql = " ".join(_RECOUNT_AGGREGATE.split()) + assert "SUM(cost_usd) FILTER (WHERE usage_source = 'measured')" in sql + assert "COUNT(*) FILTER (WHERE usage_source <> 'measured')" in sql + assert "measured_cost_usd = EXCLUDED.measured_cost_usd" in sql + assert "unmeasured_call_count = EXCLUDED.unmeasured_call_count" in sql + + async def test_duplicate_skips_recount_and_snapshot(writer_boundary) -> None: writer, _, conn = writer_boundary conn.fetch.return_value = [] diff --git a/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py new file mode 100644 index 0000000000..42cfd57ec8 --- /dev/null +++ b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py @@ -0,0 +1,189 @@ +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""OMN-20006 AC-P: the local delegate path materializes usage-by-model-day. + +The local in-process ``onex delegate`` path wrote ``delegation_events`` and +``llm_call_metrics`` but no usage rows, so the Usage page had nothing to render +on a developer machine. One successful local dispatch must leave one +``usage_by_model_day_calls`` row keyed by the run's correlation id, with the +terminal's token counts and the run's usage source, and its day-and-model +rollup row. + +Failure modes: + +* no usage row at all (the path never projects usage); +* the row's tokens or day differ from the delegation row's; +* two runs with equal tokens collapse into one call row; +* a usage write that raises breaks the delegation response, or takes the + ``llm_call_metrics`` write down with it. +""" + +from __future__ import annotations + +import asyncio +import sqlite3 +from pathlib import Path +from typing import Any +from uuid import UUID, uuid4 + +import pytest + +from omnimarket.nodes.node_delegate_skill_orchestrator.ports import ( + port_local_delegation_dispatch, +) +from omnimarket.nodes.node_delegate_skill_orchestrator.ports.port_local_delegation_dispatch import ( + LocalDelegationDispatchPort, +) +from omnimarket.nodes.node_llm_delegation_call_effect.handlers import ( + handler_llm_delegation_call, + transport, +) +from omnimarket.routing import delegation_backend_resolution + +_BACKENDS: list[dict[str, object]] = [ + { + "backend_id": "local-coder", + "endpoint_url": "http://inference.example:8000/v1/chat/completions", + "model_name": "Qwen3.6-35B-A3B", + "tier": "local", + "max_tokens": 65536, + "timeout_ms": 300000, + "capabilities": ["code_generation"], + } +] + + +@pytest.fixture(autouse=True) +def _clear_health_cache() -> None: + handler_llm_delegation_call._health_cache.clear() + + +@pytest.fixture +def _local_backend(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + delegation_backend_resolution, "load_bifrost_backends", lambda **_: _BACKENDS + ) + + def fake_post(**_: Any) -> transport.ModelTransportResponse: + return transport.ModelTransportResponse( + status_code=200, + json_body={ + "choices": [ + { + "message": { + "content": "### ANSWER\ndef reverse(s): return s[::-1]" + } + } + ], + "model": "Qwen3.6-35B-A3B", + "usage": { + "prompt_tokens": 11, + "completion_tokens": 22, + "total_tokens": 33, + }, + }, + latency_ms=42, + ) + + monkeypatch.setattr(transport, "probe_health", lambda *_, **__: True) + monkeypatch.setattr(transport, "post_chat_completion", fake_post) + + +def _dispatch(db_path: Path, correlation_id: UUID) -> dict[str, Any]: + port = LocalDelegationDispatchPort( + evidence_db_path=db_path, effect_process_boundary=False + ) + result: dict[str, Any] = asyncio.run( + port.dispatch( + prompt="reverse a string", + task_type="code_generation", + correlation_id=correlation_id, + max_tokens=256, + source_file_path=None, + source_session_id=None, + wait=True, + execution_timeout_seconds=240, + terminal_delivery_margin_seconds=60, + quality_contract_mode="extend_task_class", + acceptance_criteria=(), + tenant_id=None, + ) + ) + return result + + +def _rows(db_path: Path, sql: str) -> list[sqlite3.Row]: + conn = sqlite3.connect(str(db_path)) + try: + conn.row_factory = sqlite3.Row + return conn.execute(sql).fetchall() + finally: + conn.close() + + +@pytest.mark.usefixtures("_local_backend") +def test_local_dispatch_materializes_a_usage_call_and_its_rollup( + tmp_path: Path, +) -> None: + db_path = tmp_path / "delegation.sqlite" + correlation_id = uuid4() + assert _dispatch(db_path, correlation_id)["status"] == "completed" + + (delegation,) = _rows( + db_path, + "SELECT tokens_input, tokens_output, tenant_id, created_at, cost_usd " + "FROM delegation_events", + ) + (call,) = _rows(db_path, "SELECT * FROM usage_by_model_day_calls") + assert call["call_id"] == str(correlation_id) + assert call["model_id"] == "Qwen3.6-35B-A3B" + assert call["input_tokens"] == delegation["tokens_input"] == 11 + assert call["output_tokens"] == delegation["tokens_output"] == 22 + assert call["usage_source"] == "measured" + # The run's cost is the delegation row's, which the Overview spend reads (AC2). + assert float(call["cost_usd"]) == float(delegation["cost_usd"]) + + (rollup,) = _rows(db_path, "SELECT * FROM usage_by_model_day") + assert rollup["usage_day"] == call["usage_day"] + assert rollup["usage_day"] == str(delegation["created_at"])[:10] + assert rollup["tenant_id"] == call["tenant_id"] + assert (rollup["input_tokens"], rollup["output_tokens"]) == (11, 22) + assert rollup["call_count"] == 1 + assert rollup["unmeasured_call_count"] == 0 + assert float(rollup["measured_cost_usd"]) == float(call["cost_usd"]) + + +@pytest.mark.usefixtures("_local_backend") +def test_two_local_runs_with_equal_tokens_are_two_usage_calls( + tmp_path: Path, +) -> None: + db_path = tmp_path / "delegation.sqlite" + ids = [uuid4(), uuid4()] + for cid in ids: + _dispatch(db_path, cid) + calls = { + r["call_id"] + for r in _rows(db_path, "SELECT call_id FROM usage_by_model_day_calls") + } + assert calls == {str(c) for c in ids} + (rollup,) = _rows( + db_path, "SELECT call_count, input_tokens FROM usage_by_model_day" + ) + assert (rollup["call_count"], rollup["input_tokens"]) == (2, 22) + + +@pytest.mark.usefixtures("_local_backend") +def test_a_failing_usage_write_never_breaks_the_delegation( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + def boom(*_: object, **__: object) -> bool: + raise RuntimeError("usage store unavailable") + + monkeypatch.setattr(port_local_delegation_dispatch, "apply_usage_call", boom) + db_path = tmp_path / "delegation.sqlite" + correlation_id = uuid4() + assert _dispatch(db_path, correlation_id)["status"] == "completed" + # The delegation row and the call-metrics row still land. + assert len(_rows(db_path, "SELECT 1 FROM delegation_events")) == 1 + assert len(_rows(db_path, "SELECT 1 FROM llm_call_metrics")) == 1 + assert _rows(db_path, "SELECT 1 FROM usage_by_model_day_calls") == [] From 056af93d5026ef33d4addd026d93926f8b4cad2d Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sun, 4 Oct 2026 22:43:22 +0530 Subject: [PATCH 02/13] 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 --- contracts/OMN-20006.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/OMN-20006.yaml b/contracts/OMN-20006.yaml index abfc6509f8..e3d06211e1 100644 --- a/contracts/OMN-20006.yaml +++ b/contracts/OMN-20006.yaml @@ -29,7 +29,7 @@ emergency_bypass: justification: "" follow_up_ticket_id: "" dod_evidence: - - id: "dod-omn20006-ac1-local-usage-rows-served-per-tenant" + - id: "dod-omn20006-ac1-local-usage-rows-per-tenant" description: "One local delegate run writes its usage call and rollup rows with the terminal's tokens, equal-token runs stay apart, a failed usage write is swallowed, and the served read carries the reading tenant's rows only." source: "manual" checks: From 983de5443818df83d35ef172027dc987566e0e4e Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Sun, 4 Oct 2026 22:50:44 +0530 Subject: [PATCH 03/13] 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 --- tests/test_omn19978_usage_by_model_day.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_omn19978_usage_by_model_day.py b/tests/test_omn19978_usage_by_model_day.py index 98a247c963..79fb555aad 100644 --- a/tests/test_omn19978_usage_by_model_day.py +++ b/tests/test_omn19978_usage_by_model_day.py @@ -383,9 +383,11 @@ def test_exposure_is_bus_backed_at_the_model_day_grain() -> None: def test_exposure_is_tenant_scoped() -> None: """AC-T: a read for one tenant never carries another tenant's rows.""" contract = _contract() - (exposure,) = load_projection_exposures_from_contract( + exposures = load_projection_exposures_from_contract( contract, str(contract["name"]), CONTRACT_PATH ) + assert len(exposures) == 1 + exposure = exposures[0] assert exposure.tenant_column == "tenant_id" From 0cb94c76e5306ef2832b23a9e619b98f2ed42fc3 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Mon, 5 Oct 2026 00:06:28 +0530 Subject: [PATCH 04/13] 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 --- .../models/model_usage_call_event.py | 59 +++++++++++++++++++ .../ports/port_local_delegation_dispatch.py | 4 +- .../models/model_usage_call_event.py | 51 +--------------- 3 files changed, 62 insertions(+), 52 deletions(-) create mode 100644 src/omnimarket/models/model_usage_call_event.py diff --git a/src/omnimarket/models/model_usage_call_event.py b/src/omnimarket/models/model_usage_call_event.py new file mode 100644 index 0000000000..028a386e11 --- /dev/null +++ b/src/omnimarket/models/model_usage_call_event.py @@ -0,0 +1,59 @@ +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""Validated LLM call with an explicit event timestamp. + +Shared model (OMN-20006): the usage projection node and the local delegation +dispatch port both build it, so it lives in ``omnimarket.models`` rather than in +either node's private models package. +""" + +from datetime import UTC, datetime + +from pydantic import AliasChoices, BaseModel, ConfigDict, Field, field_validator + +from omnimarket.enums.enum_usage_source import EnumUsageSource + + +class ModelUsageCallEvent(BaseModel): + model_config = ConfigDict(frozen=True, extra="ignore", populate_by_name=True) + + call_id: str = Field( + default="", + validation_alias=AliasChoices("call_id", "correlation_id", "input_hash"), + ) + model_name: str = Field( + min_length=1, validation_alias=AliasChoices("model_name", "model_id") + ) + tenant_id: str | None = None + prompt_tokens: int = Field( + default=0, ge=0, validation_alias=AliasChoices("prompt_tokens", "input_tokens") + ) + completion_tokens: int = Field( + default=0, + ge=0, + validation_alias=AliasChoices("completion_tokens", "output_tokens"), + ) + estimated_cost_usd: float = Field( + default=0.0, + ge=0, + allow_inf_nan=False, + validation_alias=AliasChoices("estimated_cost_usd", "cost_usd"), + ) + # How the call's cost was obtained. A call that does not say is UNKNOWN, never + # assumed measured: only a measured cost may reach measured_cost_usd. + usage_source: EnumUsageSource = EnumUsageSource.UNKNOWN + session_id: str | None = None + timestamp: datetime = Field( + validation_alias=AliasChoices("timestamp", "emitted_at", "created_at") + ) + + @field_validator("timestamp") + @classmethod + def aware_timestamp(cls, value: datetime) -> datetime: + """Interpret an explicit naive timestamp as UTC, never local time.""" + if value.tzinfo is None: + return value.replace(tzinfo=UTC) + return value + + +__all__ = ["ModelUsageCallEvent"] diff --git a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py index 82459b4b25..0995c8141a 100644 --- a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py +++ b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py @@ -150,6 +150,7 @@ ModelQualityGateResult, ModelQualityRuleEvaluation, ) +from omnimarket.models.model_usage_call_event import ModelUsageCallEvent from omnimarket.nodes.node_delegate_skill_orchestrator.ports.evidence_db_resolution import ( resolve_local_delegation_evidence_db, ) @@ -220,9 +221,6 @@ from omnimarket.nodes.node_projection_usage_by_model_day.handlers.handler_usage_by_model_day_store import ( apply_usage_call, ) -from omnimarket.nodes.node_projection_usage_by_model_day.models import ( - ModelUsageCallEvent, -) from omnimarket.pricing import ModelBaselineSavings, compute_baseline_savings from omnimarket.projection.protocol_database import DatabaseAdapter from omnimarket.projection.snapshot_publisher import ModelSnapshotDeltaMessage diff --git a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py index 5b7df15b21..c82bcbaf21 100644 --- a/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py +++ b/src/omnimarket/nodes/node_projection_usage_by_model_day/models/model_usage_call_event.py @@ -1,54 +1,7 @@ # SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. # SPDX-License-Identifier: MIT -"""Validated LLM call with an explicit event timestamp.""" - -from datetime import UTC, datetime - -from pydantic import AliasChoices, BaseModel, ConfigDict, Field, field_validator - -from omnimarket.enums.enum_usage_source import EnumUsageSource - - -class ModelUsageCallEvent(BaseModel): - model_config = ConfigDict(frozen=True, extra="ignore", populate_by_name=True) - - call_id: str = Field( - default="", - validation_alias=AliasChoices("call_id", "correlation_id", "input_hash"), - ) - model_name: str = Field( - min_length=1, validation_alias=AliasChoices("model_name", "model_id") - ) - tenant_id: str | None = None - prompt_tokens: int = Field( - default=0, ge=0, validation_alias=AliasChoices("prompt_tokens", "input_tokens") - ) - completion_tokens: int = Field( - default=0, - ge=0, - validation_alias=AliasChoices("completion_tokens", "output_tokens"), - ) - estimated_cost_usd: float = Field( - default=0.0, - ge=0, - allow_inf_nan=False, - validation_alias=AliasChoices("estimated_cost_usd", "cost_usd"), - ) - # How the call's cost was obtained. A call that does not say is UNKNOWN, never - # assumed measured: only a measured cost may reach measured_cost_usd. - usage_source: EnumUsageSource = EnumUsageSource.UNKNOWN - session_id: str | None = None - timestamp: datetime = Field( - validation_alias=AliasChoices("timestamp", "emitted_at", "created_at") - ) - - @field_validator("timestamp") - @classmethod - def aware_timestamp(cls, value: datetime) -> datetime: - """Interpret an explicit naive timestamp as UTC, never local time.""" - if value.tzinfo is None: - return value.replace(tzinfo=UTC) - return value +"""Re-export: ``ModelUsageCallEvent`` now lives in ``omnimarket.models`` (OMN-20006).""" +from omnimarket.models.model_usage_call_event import ModelUsageCallEvent __all__ = ["ModelUsageCallEvent"] From 2262ee8f0a4e2906b718d71e9f1bc93aab57b3a0 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Mon, 5 Oct 2026 09:04:51 +0530 Subject: [PATCH 05/13] 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 --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index da2ef90e08..5b1e775ea2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "omnimarket" -version = "0.4.299" +version = "0.4.300" description = "OmniMarket - Portable ONEX workflow package registry" authors = [ {name = "OmniNode.ai", email = "contact@omninode.ai"} diff --git a/uv.lock b/uv.lock index 55a4f5cb60..69245278c2 100644 --- a/uv.lock +++ b/uv.lock @@ -3143,7 +3143,7 @@ wheels = [ [[package]] name = "omnimarket" -version = "0.4.299" +version = "0.4.300" source = { editable = "." } dependencies = [ { name = "aiokafka" }, From 24ff8612a73bf2c54157a8af44ef4818a507340f Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Mon, 5 Oct 2026 09:30:54 +0530 Subject: [PATCH 06/13] 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 --- src/omnimarket/projection/sqlite_database.py | 7 +++ tests/test_omn19978_usage_by_model_day.py | 48 ++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/omnimarket/projection/sqlite_database.py b/src/omnimarket/projection/sqlite_database.py index cdb15e4262..118bbb85f9 100644 --- a/src/omnimarket/projection/sqlite_database.py +++ b/src/omnimarket/projection/sqlite_database.py @@ -488,6 +488,13 @@ def _usage_by_model_day_reconciled(cls, conn: sqlite3.Connection) -> bool: def _reconcile_usage_by_model_day(cls, conn: sqlite3.Connection) -> None: if cls._usage_by_model_day_reconciled(conn): return + # The llm_call_metrics relabel earlier in _connect is an UPDATE, and the + # sqlite3 module opens a transaction before it that stays open until the + # end of _connect. BEGIN IMMEDIATE inside it raises, and while it is open + # this connection holds the write lock other first opens wait on. Commit + # it first: the relabel is idempotent and _connect commits it anyway. + if conn.in_transaction: + conn.commit() # Two first opens of one store can race: the check and the ALTERs run in # one write transaction, so the second never adds a column twice. conn.execute("BEGIN IMMEDIATE") diff --git a/tests/test_omn19978_usage_by_model_day.py b/tests/test_omn19978_usage_by_model_day.py index 79fb555aad..67ae9a31b5 100644 --- a/tests/test_omn19978_usage_by_model_day.py +++ b/tests/test_omn19978_usage_by_model_day.py @@ -300,6 +300,54 @@ def test_an_old_local_store_gains_the_new_columns_on_connect(tmp_path: Path) -> assert int(str(row["call_count"])) == 2 +def test_a_store_written_by_the_previous_release_opens_and_keeps_its_relabel( + tmp_path: Path, +) -> None: + """The store every existing install has: llm_call_metrics already carries + usage_source (so the connect-time relabel UPDATE runs and opens an implicit + transaction), and the usage tables predate the new columns and triggers. + Opening it must add the columns, and must commit the relabel, not undo it.""" + path = tmp_path / "previous-release.sqlite" + conn = sqlite3.connect(path) + conn.execute( + "CREATE TABLE llm_call_metrics (correlation_id TEXT, usage_source TEXT, " + "input_hash TEXT NOT NULL UNIQUE)" + ) + conn.execute("INSERT INTO llm_call_metrics VALUES ('c-1', 'API', 'h-1')") + conn.execute( + "CREATE TABLE usage_by_model_day_calls (call_id TEXT PRIMARY KEY, " + "tenant_id TEXT NOT NULL, usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, occurred_at TEXT NOT NULL, ingested_at TEXT NOT NULL)" + ) + conn.execute( + "CREATE TABLE usage_by_model_day (tenant_id TEXT NOT NULL, " + "usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, call_count INTEGER NOT NULL, " + "updated_at TEXT NOT NULL, PRIMARY KEY (tenant_id, usage_day, model_id))" + ) + conn.commit() + conn.close() + + assert SqliteDatabaseAdapter(path).query(AGGREGATE_TABLE) == [] + + check = sqlite3.connect(path) + try: + columns = {r[1] for r in check.execute("PRAGMA table_info(usage_by_model_day)")} + relabelled = check.execute( + "SELECT usage_source FROM llm_call_metrics WHERE input_hash = 'h-1'" + ).fetchone() + finally: + check.close() + assert { + "measured_cost_usd", + "unmeasured_call_count", + "projection_cursor", + } <= columns + assert relabelled == ("measured",) + + def test_each_insert_and_recount_takes_the_next_cursor(tmp_path: Path) -> None: """The served exposure walks by projection_cursor, so a local store must stamp one, and re-stamp a recounted row the way the Postgres BIGSERIAL does.""" From f0e1f376f26658be86c76ad899eab0f43f5a4df3 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Tue, 6 Oct 2026 10:15:06 +0530 Subject: [PATCH 07/13] 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 --- src/omnimarket/projection/sqlite_database.py | 182 +++++------ tests/test_omn19978_usage_by_model_day.py | 6 +- ..._usage_by_model_day_store_step_omn20006.py | 284 ++++++++++++++++++ .../test_omn19968_usage_source_vocabulary.py | 7 +- 4 files changed, 385 insertions(+), 94 deletions(-) create mode 100644 tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py diff --git a/src/omnimarket/projection/sqlite_database.py b/src/omnimarket/projection/sqlite_database.py index 25e1725fdb..06fe72e064 100644 --- a/src/omnimarket/projection/sqlite_database.py +++ b/src/omnimarket/projection/sqlite_database.py @@ -24,7 +24,7 @@ import json import logging import sqlite3 -from collections.abc import Mapping, Sequence +from collections.abc import Callable, Mapping, Sequence from datetime import UTC, datetime from decimal import Decimal from pathlib import Path @@ -215,9 +215,10 @@ # OMN-20006: columns the usage-by-model-day exposure serves that a store written # before them lacks. The read node refuses an exposure whose declared column the -# table lacks (projection_column_missing), so they are added on connect, with the -# same defaults as migration 0002: an old call reads 'unknown', an old aggregate -# NULL measured cost and 0 unmeasured calls until its key is recounted. +# table lacks (projection_column_missing), so they are added by a one-time store +# step on connect, with the same defaults as migration 0002: an old call reads +# 'unknown', an old aggregate NULL measured cost and 0 unmeasured calls until its +# key is recounted. _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: tuple[tuple[str, str, str], ...] = ( ("usage_by_model_day_calls", "usage_source", "TEXT NOT NULL DEFAULT 'unknown'"), ("usage_by_model_day", "measured_cost_usd", "REAL"), @@ -368,6 +369,8 @@ ) """ _USAGE_SOURCE_VOCABULARY_STEP = "omn19968_usage_source_shared_vocabulary" +# The SQLite counterpart of usage_by_model_day migration 0002. +_USAGE_BY_MODEL_DAY_STEP = "omn20006_usage_by_model_day_measured_cost" # JSON-serialized columns: list/dict values are stored as TEXT JSON so the # sqlite row round-trips structurally for evidence queries. @@ -437,7 +440,6 @@ def _connect(self) -> sqlite3.Connection: conn.execute(_DELEGATE_SKILL_CLAIMS_DDL) conn.execute(_USAGE_BY_MODEL_DAY_CALLS_DDL) conn.execute(_USAGE_BY_MODEL_DAY_DDL) - self._reconcile_usage_by_model_day(conn) conn.execute(_METERING_SUMMARY_DDL) conn.execute(_METERING_SUMMARY_INDEX_DDL) conn.execute(_LLM_CALL_METRICS_DDL) @@ -445,7 +447,7 @@ def _connect(self) -> sqlite3.Connection: conn.execute(_TENANT_INFERENCE_CREDENTIALS_DDL) conn.execute(_DELEGATION_ROUTING_TENANT_OVERLAY_DDL) conn.commit() - self._apply_usage_source_vocabulary_step(conn, self._db_path) + self._apply_store_steps(conn, self._db_path) return conn @staticmethod @@ -489,47 +491,6 @@ def _reconcile_delegation_events(cls, conn: sqlite3.Connection) -> None: conn.rollback() raise - @classmethod - def _usage_by_model_day_reconciled(cls, conn: sqlite3.Connection) -> bool: - for table, column, _ in _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: - if column not in cls._existing_columns(conn, table): - return False - triggers = conn.execute( - "SELECT COUNT(*) FROM sqlite_master WHERE type = 'trigger' AND name IN " - "('usage_by_model_day_cursor_on_insert', " - "'usage_by_model_day_cursor_on_recount')" - ).fetchone()[0] - return bool(triggers == len(_USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS)) - - @classmethod - def _reconcile_usage_by_model_day(cls, conn: sqlite3.Connection) -> None: - if cls._usage_by_model_day_reconciled(conn): - return - # The llm_call_metrics relabel earlier in _connect is an UPDATE, and the - # sqlite3 module opens a transaction before it that stays open until the - # end of _connect. BEGIN IMMEDIATE inside it raises, and while it is open - # this connection holds the write lock other first opens wait on. Commit - # it first: the relabel is idempotent and _connect commits it anyway. - if conn.in_transaction: - conn.commit() - # Two first opens of one store can race: the check and the ALTERs run in - # one write transaction, so the second never adds a column twice. - conn.execute("BEGIN IMMEDIATE") - try: - for table, column, declaration in _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: - if column not in cls._existing_columns(conn, table): - conn.execute( - f"ALTER TABLE {table} ADD COLUMN {column} {declaration}" - ) - conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_DDL) - conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_SEED) - for trigger in _USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS: - conn.execute(trigger) - conn.commit() - except BaseException: - conn.rollback() - raise - @classmethod def _reconcile_legacy_llm_call_metrics(cls, conn: sqlite3.Connection) -> None: columns = cls._existing_columns(conn, "llm_call_metrics") @@ -556,53 +517,94 @@ def _store_step_recorded(conn: sqlite3.Connection, step: str) -> bool: ) @classmethod - def _apply_usage_source_vocabulary_step( - cls, conn: sqlite3.Connection, db_path: Path - ) -> None: - """OMN-19968: move rows written before the shared vocabulary onto it, once. + def _apply_store_steps(cls, conn: sqlite3.Connection, db_path: Path) -> None: + """Run each one-time store step this store has not recorded, in order. + + Every connection goes through here, reads included, and a step's write + takes the write lock even when it changes nothing. So each step runs in + its own write transaction that commits it together with its row in the + store-steps table, and a store that has recorded every step opens with + reads only. The decision reads the step record, never table rows: this + module is shared by nodes that do not own those tables. + + A store this process cannot write is read as it is, with one warning + naming every step it still lacks. Anything else, a busy store included, + is a real failure and raises. + """ + pending: list[tuple[str, Callable[[sqlite3.Connection], None], str]] = [] + if not cls._store_step_recorded( + conn, _USAGE_SOURCE_VOCABULARY_STEP + ) and "usage_source" in cls._existing_columns(conn, "llm_call_metrics"): + pending.append( + ( + _USAGE_SOURCE_VOCABULARY_STEP, + cls._relabel_usage_source, + "moved llm_call_metrics.usage_source onto the shared vocabulary", + ) + ) + if not cls._store_step_recorded(conn, _USAGE_BY_MODEL_DAY_STEP): + pending.append( + ( + _USAGE_BY_MODEL_DAY_STEP, + cls._add_usage_by_model_day_columns, + "added the usage-by-model-day columns and cursor triggers", + ) + ) + for index, (step, apply, _) in enumerate(pending): + try: + conn.execute("BEGIN IMMEDIATE") + conn.execute(_STORE_STEPS_DDL) + apply(conn) + conn.execute( + f"INSERT OR IGNORE INTO {_STORE_STEPS_TABLE} (step, applied_at) " + "VALUES (?, ?)", + (step, datetime.now(UTC).isoformat()), + ) + conn.commit() + except BaseException as exc: + conn.rollback() + if not ( + isinstance(exc, sqlite3.OperationalError) + and (exc.sqlite_errorcode & 0xFF) == sqlite3.SQLITE_READONLY + ): + raise + logger.warning( + "%s is read-only and has not %s; reading it as it is", + db_path, + ", nor ".join(lacking for _, _, lacking in pending[index:]), + ) + return + + @staticmethod + def _relabel_usage_source(conn: sqlite3.Connection) -> None: + """OMN-19968: move rows written before the shared vocabulary onto it. The SQLite counterpart of migration 0003 (EnumUsageSource; omnibase_infra - migration 077). It runs once per store, and the relabel and its record - commit together. A store that has recorded it opens with reads only. - Every connection goes through here, reads included, and an UPDATE takes a - write lock even when it matches nothing. + migration 077), run once per store as a store step. + """ + conn.execute( + "UPDATE llm_call_metrics SET usage_source = CASE usage_source " + "WHEN 'API' THEN 'measured' WHEN 'ESTIMATED' THEN 'estimated' " + "WHEN 'MISSING' THEN 'unknown' ELSE usage_source END " + "WHERE usage_source IN ('API', 'ESTIMATED', 'MISSING')" + ) + + @classmethod + def _add_usage_by_model_day_columns(cls, conn: sqlite3.Connection) -> None: + """OMN-20006: give a store written before them the usage-by-model-day + columns and the two cursor triggers, run once per store as a store step. - The decision reads the store's step record, never llm_call_metrics rows: - this module is shared by nodes that do not own that table. + The columns are read again here, under the step's write lock, so two + first opens of one store that race never add a column twice; a fresh + store's tables already have them and only gain the triggers. """ - if cls._store_step_recorded(conn, _USAGE_SOURCE_VOCABULARY_STEP): - return - if "usage_source" not in cls._existing_columns(conn, "llm_call_metrics"): - return - try: - conn.execute("BEGIN IMMEDIATE") - conn.execute(_STORE_STEPS_DDL) - conn.execute( - "UPDATE llm_call_metrics SET usage_source = CASE usage_source " - "WHEN 'API' THEN 'measured' WHEN 'ESTIMATED' THEN 'estimated' " - "WHEN 'MISSING' THEN 'unknown' ELSE usage_source END " - "WHERE usage_source IN ('API', 'ESTIMATED', 'MISSING')" - ) - conn.execute( - f"INSERT OR IGNORE INTO {_STORE_STEPS_TABLE} (step, applied_at) " - "VALUES (?, ?)", - (_USAGE_SOURCE_VOCABULARY_STEP, datetime.now(UTC).isoformat()), - ) - conn.commit() - except sqlite3.Error as exc: - conn.rollback() - # A store this process cannot write is still read, with the labels - # it holds. Anything else, a busy store included, is a real failure. - if not ( - isinstance(exc, sqlite3.OperationalError) - and (exc.sqlite_errorcode & 0xFF) == sqlite3.SQLITE_READONLY - ): - raise - logger.warning( - "%s is read-only and has not moved llm_call_metrics.usage_source " - "onto the shared vocabulary; reading it as it is", - db_path, - ) + for table, column, declaration in _USAGE_BY_MODEL_DAY_ADDED_COLUMNS: + if column not in cls._existing_columns(conn, table): + conn.execute(f"ALTER TABLE {table} ADD COLUMN {column} {declaration}") + conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_DDL) + conn.execute(_USAGE_BY_MODEL_DAY_CURSOR_SEQ_SEED) + for trigger in _USAGE_BY_MODEL_DAY_CURSOR_TRIGGERS: + conn.execute(trigger) @staticmethod def _existing_columns(conn: sqlite3.Connection, table: str) -> set[str]: diff --git a/tests/test_omn19978_usage_by_model_day.py b/tests/test_omn19978_usage_by_model_day.py index 67ae9a31b5..979c513b77 100644 --- a/tests/test_omn19978_usage_by_model_day.py +++ b/tests/test_omn19978_usage_by_model_day.py @@ -304,9 +304,9 @@ def test_a_store_written_by_the_previous_release_opens_and_keeps_its_relabel( tmp_path: Path, ) -> None: """The store every existing install has: llm_call_metrics already carries - usage_source (so the connect-time relabel UPDATE runs and opens an implicit - transaction), and the usage tables predate the new columns and triggers. - Opening it must add the columns, and must commit the relabel, not undo it.""" + usage_source with a retired label, and the usage tables predate the new + columns and triggers, so both one-time store steps are pending. Opening it + must add the columns, and must commit the relabel, not undo it.""" path = tmp_path / "previous-release.sqlite" conn = sqlite3.connect(path) conn.execute( diff --git a/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py b/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py new file mode 100644 index 0000000000..2b5e461243 --- /dev/null +++ b/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py @@ -0,0 +1,284 @@ +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""The usage-by-model-day columns and cursor triggers are a one-time store step. + +A local store written before the usage-by-model-day exposure lacks three +aggregate columns, the calls table's ``usage_source`` and the two cursor +triggers. ``SqliteDatabaseAdapter`` adds them on connect, the way it moves +``llm_call_metrics.usage_source`` onto the shared vocabulary: once per store, +committed together with its row in the store-steps table, and skipped by every +later connection, reads included. + +Each test names the failure it exists to catch: + +* a read-only store written before the change raises on every read, of every + table, instead of being read as it is (the reconcile's write is not optional + for a store this process cannot write); +* the same, for a store whose folder is read-only (SQLite cannot create its + rollback journal; the extended code is SQLITE_READONLY_DIRECTORY); +* the step adds the columns but is not recorded, or is recorded but a later + read still takes the write lock; +* the columns are added but the step record fails, so a half-done step is + committed and never retried as a whole; +* a busy writable store is passed off as read-only and read without the + columns; +* two first opens of one store race and the second adds a column twice. + +Every store is a SQLite file under pytest's ``tmp_path``. +""" + +from __future__ import annotations + +import logging +import os +import sqlite3 +import threading +from collections.abc import Iterator +from contextlib import closing, contextmanager +from pathlib import Path +from typing import Any + +import pytest + +from omnimarket.projection import sqlite_database +from omnimarket.projection.sqlite_database import SqliteDatabaseAdapter + +pytestmark = pytest.mark.unit + +# The adapter's bookkeeping, named here as literals rather than imported so +# these tests also run against code that predates the usage step. +STEPS_TABLE = "omnimarket_sqlite_store_steps" +USAGE_STEP = "omn20006_usage_by_model_day_measured_cost" +ADDED_COLUMNS = { + "usage_by_model_day_calls": {"usage_source"}, + "usage_by_model_day": { + "measured_cost_usd", + "unmeasured_call_count", + "projection_cursor", + }, +} +CURSOR_TRIGGERS = { + "usage_by_model_day_cursor_on_insert", + "usage_by_model_day_cursor_on_recount", +} + + +def _store_written_before_the_change(path: Path) -> None: + """A store as the release before this change leaves it: every other step + applied (the usage-source vocabulary step recorded), the usage tables in + their first shape, no cursor triggers, no usage step recorded.""" + with closing(SqliteDatabaseAdapter(path)._connect()) as conn: + conn.execute( + "INSERT INTO llm_call_metrics (model_id, input_hash, usage_source) " + "VALUES ('m', 'h-1', 'measured')" + ) + for trigger in CURSOR_TRIGGERS: + conn.execute(f"DROP TRIGGER IF EXISTS {trigger}") + conn.execute("DROP TABLE IF EXISTS usage_by_model_day_projection_cursor_seq") + conn.execute("DROP TABLE usage_by_model_day_calls") + conn.execute("DROP TABLE usage_by_model_day") + conn.execute( + "CREATE TABLE usage_by_model_day_calls (call_id TEXT PRIMARY KEY, " + "tenant_id TEXT NOT NULL, usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, occurred_at TEXT NOT NULL, ingested_at TEXT NOT NULL)" + ) + conn.execute( + "CREATE TABLE usage_by_model_day (tenant_id TEXT NOT NULL, " + "usage_day TEXT NOT NULL, model_id TEXT NOT NULL, " + "input_tokens INTEGER NOT NULL, output_tokens INTEGER NOT NULL, " + "cost_usd REAL NOT NULL, call_count INTEGER NOT NULL, " + "updated_at TEXT NOT NULL, PRIMARY KEY (tenant_id, usage_day, model_id))" + ) + conn.execute(f"DELETE FROM {STEPS_TABLE} WHERE step = ?", (USAGE_STEP,)) + conn.commit() + + +def _missing(path: Path) -> dict[str, set[str]]: + """The added columns and triggers the file lacks, read past the adapter.""" + with closing(sqlite3.connect(path)) as conn: + missing = { + table: columns + - {str(r[1]) for r in conn.execute(f"PRAGMA table_info({table})")} + for table, columns in ADDED_COLUMNS.items() + } + missing["triggers"] = CURSOR_TRIGGERS - { + str(r[0]) + for r in conn.execute( + "SELECT name FROM sqlite_master WHERE type = 'trigger'" + ) + } + return {k: v for k, v in missing.items() if v} + + +def _steps(path: Path) -> set[str]: + with closing(sqlite3.connect(path)) as conn: + return {str(r[0]) for r in conn.execute(f"SELECT step FROM {STEPS_TABLE}")} + + +@contextmanager +def _read_only(path: Path) -> Iterator[None]: + path.chmod(0o444) + path.parent.chmod(0o555) + try: + if os.access(path, os.W_OK): + pytest.fail( + f"{path} is still writable with mode 0o444 (running as root?); " + "this test cannot prove a read-only store is read" + ) + yield + finally: + path.parent.chmod(0o755) + path.chmod(0o644) + + +@contextmanager +def _held_by_another_writer( + path: Path, monkeypatch: pytest.MonkeyPatch +) -> Iterator[None]: + """Another connection holds a write transaction; the adapter does not wait, + so a write lock taken on connect shows as an error, not a 5 s stall.""" + connect = sqlite3.connect + + def connect_without_waiting( + database: Path, *args: Any, **kwargs: Any + ) -> sqlite3.Connection: + conn: sqlite3.Connection = connect(database, *args, **{**kwargs, "timeout": 0}) + return conn + + writer = connect(path, isolation_level=None) + writer.execute("BEGIN IMMEDIATE") + # The adapter calls sqlite3.connect through the module it imports. + monkeypatch.setattr(sqlite3, "connect", connect_without_waiting) + try: + yield + finally: + monkeypatch.undo() + writer.rollback() + writer.close() + + +def _warnings(caplog: pytest.LogCaptureFixture) -> list[logging.LogRecord]: + return [ + record + for record in caplog.records + if record.name == sqlite_database.__name__ and record.levelno == logging.WARNING + ] + + +def test_a_read_only_store_written_before_the_change_is_read_as_it_is( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + caplog.set_level(logging.WARNING, logger=sqlite_database.__name__) + + with _read_only(path): + rows = SqliteDatabaseAdapter(path).query("llm_call_metrics") + + assert [row["input_hash"] for row in rows] == ["h-1"] + assert _missing(path) == {**ADDED_COLUMNS, "triggers": CURSOR_TRIGGERS} + warnings = _warnings(caplog) + assert len(warnings) == 1 + assert str(path) in warnings[0].getMessage() + + +def test_a_store_in_a_read_only_folder_written_before_the_change_is_read_as_it_is( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + caplog.set_level(logging.WARNING, logger=sqlite_database.__name__) + + path.parent.chmod(0o555) + try: + if os.access(path.parent, os.W_OK): + pytest.fail( + f"{path.parent} is still writable with mode 0o555 (running as " + "root?); this test cannot prove a store in a read-only folder is read" + ) + rows = SqliteDatabaseAdapter(path).query("llm_call_metrics") + finally: + path.parent.chmod(0o755) + + assert [row["input_hash"] for row in rows] == ["h-1"] + warnings = _warnings(caplog) + assert len(warnings) == 1 + assert str(path) in warnings[0].getMessage() + + +def test_the_first_open_applies_and_records_the_step_and_a_later_read_takes_no_lock( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + + SqliteDatabaseAdapter(path).query("usage_by_model_day") + + assert _missing(path) == {} + assert USAGE_STEP in _steps(path) + with _held_by_another_writer(path, monkeypatch): + rows = SqliteDatabaseAdapter(path).query("llm_call_metrics") + assert [row["input_hash"] for row in rows] == ["h-1"] + + +def test_a_failed_step_record_leaves_the_columns_unadded(tmp_path: Path) -> None: + # The columns, the triggers and the step's record are one transaction: if + # the record cannot be written, nothing else may stay behind either, so the + # next connect retries the whole step rather than trusting a half-done one. + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + with closing(sqlite3.connect(path)) as conn: + conn.execute( + f"CREATE TRIGGER refuse_usage_step BEFORE INSERT ON {STEPS_TABLE} " + f"WHEN NEW.step = '{USAGE_STEP}' " + "BEGIN SELECT RAISE(ABORT, 'step record refused'); END" + ) + conn.commit() + + with pytest.raises(sqlite3.IntegrityError, match="step record refused"): + SqliteDatabaseAdapter(path).query("usage_by_model_day") + + assert _missing(path) == {**ADDED_COLUMNS, "triggers": CURSOR_TRIGGERS} + assert USAGE_STEP not in _steps(path) + + +def test_a_locked_store_written_before_the_change_raises_rather_than_reading( + tmp_path: Path, caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + caplog.set_level(logging.WARNING, logger=sqlite_database.__name__) + + with ( + _held_by_another_writer(path, monkeypatch), + pytest.raises(sqlite3.OperationalError, match="locked"), + ): + SqliteDatabaseAdapter(path).query("usage_by_model_day") + + assert _warnings(caplog) == [] + assert USAGE_STEP not in _steps(path) + + +def test_two_first_opens_together_add_each_column_once(tmp_path: Path) -> None: + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + barrier = threading.Barrier(2) + errors: list[BaseException] = [] + + def open_store() -> None: + barrier.wait() + try: + SqliteDatabaseAdapter(path).query("usage_by_model_day") + except BaseException as exc: + errors.append(exc) + + threads = [threading.Thread(target=open_store) for _ in range(2)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + assert errors == [] + assert _missing(path) == {} + assert USAGE_STEP in _steps(path) diff --git a/tests/unit/test_omn19968_usage_source_vocabulary.py b/tests/unit/test_omn19968_usage_source_vocabulary.py index 47d180ceff..9786def5b0 100644 --- a/tests/unit/test_omn19968_usage_source_vocabulary.py +++ b/tests/unit/test_omn19968_usage_source_vocabulary.py @@ -124,6 +124,8 @@ def _labels(path: Path) -> list[str]: def _step_rows(path: Path) -> list[tuple[str, str]]: + """The vocabulary step's record, the only step these tests are about; the + store records its other one-time steps (OMN-20006's) in the same table.""" with closing(sqlite3.connect(path)) as conn: exists = conn.execute( "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = ?", @@ -133,7 +135,10 @@ def _step_rows(path: Path) -> list[tuple[str, str]]: return [] return [ (str(r[0]), str(r[1])) - for r in conn.execute(f"SELECT step, applied_at FROM {STEPS_TABLE}") + for r in conn.execute( + f"SELECT step, applied_at FROM {STEPS_TABLE} WHERE step = ?", + (VOCAB_STEP,), + ) ] From 8307a16632b364c8121e8aa9bdf563b74fe6b8ad Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Tue, 6 Oct 2026 14:16:20 +0530 Subject: [PATCH 08/13] 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 : in attempt order. Onex-Lane: omn20006-usage-page Onex-Session: d99c149d-35af-4f1e-b791-28a28187bfc4 --- .../ports/port_local_delegation_dispatch.py | 148 +++++++++--- .../test_local_dispatch_usage_by_model_day.py | 216 +++++++++++++++++- 2 files changed, 337 insertions(+), 27 deletions(-) diff --git a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py index 4d842555a3..d73328f2e0 100644 --- a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py +++ b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py @@ -51,6 +51,8 @@ import time import uuid from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass +from datetime import datetime from decimal import Decimal from pathlib import Path from typing import Any, Literal, cast @@ -1224,6 +1226,10 @@ async def dispatch( # a rejected metered tier's real cost is never dropped (bus # ``_bank_attempt_spend`` parity). Projected as the row's cost_usd. cumulative_cost_usd = Decimal("0") + # OMN-20006: what each banked call consumed, on the model that served + # it, so the usage rows split the run's cost per model. Appended + # wherever cumulative_cost_usd is, so the two always agree. + attempt_usage: list[_AttemptUsage] = [] attempts: list[dict[str, object]] = [] escalation_count = 0 # OMN-14220: best authored artifact seen across attempts (highest gate score, @@ -1507,6 +1513,9 @@ async def dispatch( # (typically zero) metered cost directly — mirrors the # post-success banking below without requiring a gate verdict. cumulative_cost_usd += transport_result.actual_cost_usd + attempt_usage.append( + _AttemptUsage.of(transport_result, model_id=backend.model_id) + ) attempts.append( { "tier": current_tier, @@ -1621,6 +1630,7 @@ async def dispatch( baseline_savings=None, escalation_count=escalation_count, attempts=attempts, + attempt_usage=attempt_usage, # Transport failure: the gate never ran, so nothing was scored. actual_score=None, required_bar=None, @@ -1685,6 +1695,11 @@ async def dispatch( # BEFORE deciding pass/fail so a rejected metered tier's spend is # counted even if we escalate away from it (OMN-13849). cumulative_cost_usd += result.actual_cost_usd + attempt_usage.append( + _AttemptUsage.of( + result, model_id=result.served_model_id or backend.model_id + ) + ) attempt_tier = _routing_tier_name(backend) @@ -1895,6 +1910,7 @@ async def dispatch( baseline_savings=baseline_savings, escalation_count=escalation_count, attempts=attempts, + attempt_usage=attempt_usage, actual_score=gate_result.quality_score, required_bar=_declared_required_bar(task_type), ) @@ -2115,6 +2131,7 @@ async def dispatch( baseline_savings=None, escalation_count=escalation_count, attempts=attempts, + attempt_usage=attempt_usage, actual_score=gate_result.quality_score, required_bar=_declared_required_bar(task_type), ) @@ -3201,6 +3218,7 @@ def _project_evidence( attempts: Sequence[Mapping[str, object]], actual_score: float | None, required_bar: float | None, + attempt_usage: Sequence[_AttemptUsage] = (), ) -> None: """Materialize a delegation_events row via the canonical projection. @@ -3223,6 +3241,12 @@ def _project_evidence( routed every local terminal through the text fallback, where a run whose rungs answered and were refused on quality could be recorded as a provider quota failure. + + OMN-20006: ``attempt_usage`` is what each banked call consumed, in + order. It becomes one usage-by-model-day call per call that reached a + provider, on that call's own model and with its own usage source, so an + escalated run's cost splits per model and still sums to ``cost_usd``. + Empty (the default, for callers that project no usage) writes none. """ payload: dict[str, object] = { "status": "completed" if quality_passed else "failed", @@ -3387,32 +3411,14 @@ def _project_evidence( correlation_id, exc_info=True, ) - # OMN-20006: the same call, folded into the usage-by-model-day tables - # the Usage page reads, keyed by this run's correlation id and dated - # by the terminal's own emitted_at, so its UTC day is the delegation - # row's. Its own guard: a failed usage write must neither break the - # response nor take the call-metrics row down with it. - try: - usage_event = ModelUsageCallEvent( - call_id=str(correlation_id), - model_name=model_id, - tenant_id=tenant_id, - prompt_tokens=result.tokens_in, - completion_tokens=result.tokens_out, - estimated_cost_usd=float(cost_usd), - usage_source=EnumUsageSource.MEASURED, - timestamp=terminal.emitted_at, - ) - apply_usage_call( - HandlerProjectionUsageByModelDay().handle(usage_event), - self._evidence_db, - ) - except Exception: - logger.warning( - "Failed to project local usage-by-model-day for correlation_id=%s", - correlation_id, - exc_info=True, - ) + # OMN-20006: the run's calls, each on its own model, folded into the + # usage-by-model-day tables the Usage page reads. + self._project_usage( + correlation_id=correlation_id, + tenant_id=tenant_id, + emitted_at=terminal.emitted_at, + attempt_usage=attempt_usage, + ) except Exception: logger.warning( "Failed to project local delegation evidence for correlation_id=%s", @@ -3428,6 +3434,61 @@ def _project_evidence( # correlation_id. self._publish_terminal(payload, quality_passed=quality_passed) + def _project_usage( + self, + *, + correlation_id: UUID, + tenant_id: str, + emitted_at: datetime, + attempt_usage: Sequence[_AttemptUsage], + ) -> None: + """Fold each call into the usage-by-model-day tables (OMN-20006). + + One call per attempt that reached a provider, on the model that served + it, with its own tokens, cost and usage source: an escalated run's + spend is split per model, and an attempt whose cost was not measured is + never booked as measured because a later one was. A call that carried + no usage at all (refused before a provider answered) is not a usage + call; its banked cost is zero, so the per-model sum still equals the + run's cost. A single call keeps the run's correlation id as its key; + several take ``:`` in attempt order. + + Dated by the terminal's own emitted_at, so its UTC day is the + delegation row's. Each call has its own guard: a failed usage write + must neither break the response nor take the call-metrics row, or + another attempt's usage, down with it. + """ + calls = [usage for usage in attempt_usage if usage.carries_usage] + for ordinal, usage in enumerate(calls, start=1): + call_id = ( + str(correlation_id) + if len(calls) == 1 + else f"{correlation_id}:{ordinal}" + ) + try: + usage_event = ModelUsageCallEvent( + call_id=call_id, + model_name=usage.model_id, + tenant_id=tenant_id, + prompt_tokens=usage.tokens_in, + completion_tokens=usage.tokens_out, + estimated_cost_usd=float(usage.cost_usd), + usage_source=usage.usage_source, + timestamp=emitted_at, + ) + apply_usage_call( + HandlerProjectionUsageByModelDay().handle(usage_event), + self._evidence_db, + ) + except Exception: + logger.warning( + "Failed to project local usage-by-model-day for " + "correlation_id=%s call_id=%s", + correlation_id, + call_id, + exc_info=True, + ) + def _publish_terminal( self, payload: Mapping[str, object], *, quality_passed: bool ) -> None: @@ -3462,6 +3523,41 @@ def _local_terminal_topic(*, success: bool) -> str | None: return str(topic) +@dataclass(frozen=True, slots=True) +class _AttemptUsage: + """What one banked effect call consumed, on the model that served it.""" + + model_id: str + tokens_in: int + tokens_out: int + cost_usd: Decimal + usage_source: EnumUsageSource + answered: bool + + @classmethod + def of( + cls, result: ModelLlmDelegationCallResult, *, model_id: str + ) -> _AttemptUsage: + return cls( + model_id=model_id, + tokens_in=result.tokens_in, + tokens_out=result.tokens_out, + cost_usd=result.actual_cost_usd, + usage_source=result.usage_source, + answered=result.success, + ) + + @property + def carries_usage(self) -> bool: + """A provider answered, or the call still recorded tokens or cost.""" + return ( + self.answered + or self.cost_usd != 0 + or self.tokens_in != 0 + or self.tokens_out != 0 + ) + + class _AttemptOutcome: """Outcome of one dispatch attempt (effect + gate) for the escalation loop. diff --git a/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py index 42cfd57ec8..26676b80a4 100644 --- a/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py +++ b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py @@ -15,30 +15,51 @@ * the row's tokens or day differ from the delegation row's; * two runs with equal tokens collapse into one call row; * a usage write that raises breaks the delegation response, or takes the - ``llm_call_metrics`` write down with it. + ``llm_call_metrics`` write down with it; +* an escalated run books every attempt's spend to the final model, so the + per-model split is wrong although the run's total is right; +* an attempt whose cost was not measured is booked as measured because a later + attempt's was; +* a call that never reached a provider (no tokens, no cost) is counted as an + unmeasured call. """ from __future__ import annotations import asyncio import sqlite3 +from collections.abc import Callable +from decimal import Decimal from pathlib import Path from typing import Any from uuid import UUID, uuid4 import pytest +from omnimarket.enums.enum_delegation_failure_class import EnumDelegationFailureClass +from omnimarket.enums.enum_usage_source import EnumUsageSource from omnimarket.nodes.node_delegate_skill_orchestrator.ports import ( port_local_delegation_dispatch, ) from omnimarket.nodes.node_delegate_skill_orchestrator.ports.port_local_delegation_dispatch import ( LocalDelegationDispatchPort, ) +from omnimarket.nodes.node_llm_delegation_call_effect import ( + ModelLlmDelegationCallRequest, + ModelLlmDelegationCallResult, +) from omnimarket.nodes.node_llm_delegation_call_effect.handlers import ( handler_llm_delegation_call, transport, ) from omnimarket.routing import delegation_backend_resolution +from tests.unit.nodes.node_delegate_skill_orchestrator.test_local_dispatch_escalation_omn13849 import ( + _dispatch as _ladder_dispatch, +) +from tests.unit.nodes.node_delegate_skill_orchestrator.test_local_dispatch_escalation_omn13849 import ( + _install_ladder, + _PerTierEffect, +) _BACKENDS: list[dict[str, object]] = [ { @@ -187,3 +208,196 @@ def boom(*_: object, **__: object) -> bool: assert len(_rows(db_path, "SELECT 1 FROM delegation_events")) == 1 assert len(_rows(db_path, "SELECT 1 FROM llm_call_metrics")) == 1 assert _rows(db_path, "SELECT 1 FROM usage_by_model_day_calls") == [] + + +# --- Escalated runs: each attempt is booked to its own model ------------------- +# The ladder from the OMN-13849 escalation tests: local (Qwen3.6-35B-A3B) -> +# cheap_cloud (glm-5.2) -> claude (gemini-2.5-flash). Each tier answers with its +# own tokens and its own metered cost, so a split that books one attempt's +# figures to another model is visible. +_TIER_TOKENS: dict[str, tuple[int, int]] = { + "local": (11, 22), + "cheap_cloud": (33, 44), + "claude": (55, 66), +} +_TIER_COST: dict[str, Decimal] = { + "local": Decimal("0.001"), + "cheap_cloud": Decimal("0.010"), + "claude": Decimal("0.030"), +} + + +class _MeteredTierEffect: + """Answer every tier, with that tier's tokens, cost and usage source.""" + + def __init__( + self, + pass_tiers: frozenset[str], + unmeasured_tiers: frozenset[str] = frozenset(), + ) -> None: + self._pass_tiers = pass_tiers + self._unmeasured_tiers = unmeasured_tiers + + def __call__( + self, request: ModelLlmDelegationCallRequest + ) -> ModelLlmDelegationCallResult: + tier = request.model_tier + tokens_in, tokens_out = _TIER_TOKENS[tier] + return ModelLlmDelegationCallResult( + request_id=request.request_id, + success=True, + content=( + _PerTierEffect._GOOD_RESEARCH + if tier in self._pass_tiers + else _PerTierEffect._REFUSAL + ), + tokens_in=tokens_in, + tokens_out=tokens_out, + latency_ms=5, + actual_cost_usd=_TIER_COST[tier], + usage_source=( + EnumUsageSource.UNKNOWN + if tier in self._unmeasured_tiers + else EnumUsageSource.MEASURED + ), + ) + + +def _ladder_run( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + effect: Callable[[ModelLlmDelegationCallRequest], ModelLlmDelegationCallResult], +) -> tuple[Path, UUID, dict[str, object]]: + _install_ladder(monkeypatch, max_escalations=2) + db_path = tmp_path / "delegation.sqlite" + port = LocalDelegationDispatchPort( + effect_handler=effect, + evidence_db_path=db_path, + effect_process_boundary=False, + ) + correlation_id = uuid4() + result = _ladder_dispatch(port, task_type="research", correlation_id=correlation_id) + return db_path, correlation_id, result + + +def _rollups_by_model(db_path: Path) -> dict[str, sqlite3.Row]: + return { + r["model_id"]: r for r in _rows(db_path, "SELECT * FROM usage_by_model_day") + } + + +def _delegation_cost(db_path: Path) -> float: + (row,) = _rows(db_path, "SELECT cost_usd FROM delegation_events") + return float(row["cost_usd"]) + + +def test_an_escalated_run_books_each_attempt_to_its_own_model( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + db_path, correlation_id, result = _ladder_run( + tmp_path, + monkeypatch, + _MeteredTierEffect(pass_tiers=frozenset({"cheap_cloud"})), + ) + assert result["status"] == "completed" + assert result["model_name"] == "glm-5.2" + + calls = _rows( + db_path, + "SELECT call_id, model_id, input_tokens, output_tokens, cost_usd, " + "usage_source FROM usage_by_model_day_calls ORDER BY call_id", + ) + assert [ + (c["model_id"], c["input_tokens"], c["output_tokens"], c["usage_source"]) + for c in calls + ] == [ + ("Qwen3.6-35B-A3B", 11, 22, "measured"), + ("glm-5.2", 33, 44, "measured"), + ] + assert [float(c["cost_usd"]) for c in calls] == pytest.approx([0.001, 0.010]) + # One run, two calls: distinct keys, both traceable to the run. + assert len({c["call_id"] for c in calls}) == 2 + assert all(c["call_id"].startswith(str(correlation_id)) for c in calls) + + rollups = _rollups_by_model(db_path) + assert set(rollups) == {"Qwen3.6-35B-A3B", "glm-5.2"} + assert float(rollups["Qwen3.6-35B-A3B"]["measured_cost_usd"]) == pytest.approx( + 0.001 + ) + assert float(rollups["glm-5.2"]["measured_cost_usd"]) == pytest.approx(0.010) + # The split preserves the run's total, which the Overview spend reads (AC2). + total = sum(float(r["measured_cost_usd"]) for r in rollups.values()) + assert total == pytest.approx(_delegation_cost(db_path)) + run_cost = result["cost_usd"] + assert isinstance(run_cost, float) + assert total == pytest.approx(run_cost) + + +def test_a_failed_run_on_the_full_ladder_books_three_models_and_its_total( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + db_path, _, result = _ladder_run( + tmp_path, monkeypatch, _MeteredTierEffect(pass_tiers=frozenset()) + ) + assert result["status"] == "failed" + rollups = _rollups_by_model(db_path) + assert {m: float(r["measured_cost_usd"]) for m, r in rollups.items()} == ( + pytest.approx( + {"Qwen3.6-35B-A3B": 0.001, "glm-5.2": 0.010, "gemini-2.5-flash": 0.030} + ) + ) + assert sum(float(r["cost_usd"]) for r in rollups.values()) == pytest.approx( + _delegation_cost(db_path) + ) + + +def test_an_unmeasured_attempt_is_never_booked_as_measured( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + db_path, _, _ = _ladder_run( + tmp_path, + monkeypatch, + _MeteredTierEffect( + pass_tiers=frozenset({"cheap_cloud"}), + unmeasured_tiers=frozenset({"local"}), + ), + ) + rollups = _rollups_by_model(db_path) + local = rollups["Qwen3.6-35B-A3B"] + assert local["measured_cost_usd"] is None + assert local["unmeasured_call_count"] == 1 + glm = rollups["glm-5.2"] + assert float(glm["measured_cost_usd"]) == pytest.approx(0.010) + assert glm["unmeasured_call_count"] == 0 + # cost_usd keeps its meaning (every call), so the run's total still adds up. + assert sum(float(r["cost_usd"]) for r in rollups.values()) == pytest.approx( + _delegation_cost(db_path) + ) + + +def test_a_call_that_reached_no_provider_is_not_a_usage_call( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + measured = _MeteredTierEffect(pass_tiers=frozenset({"cheap_cloud"})) + + def refused_then_ok( + request: ModelLlmDelegationCallRequest, + ) -> ModelLlmDelegationCallResult: + if request.model_tier == "local": + return ModelLlmDelegationCallResult( + request_id=request.request_id, + success=False, + failure_class=EnumDelegationFailureClass.MODEL_UNAVAILABLE, + error_message="connection refused", + ) + return measured(request) + + db_path, correlation_id, result = _ladder_run( + tmp_path, monkeypatch, refused_then_ok + ) + assert result["status"] == "completed" + (call,) = _rows(db_path, "SELECT * FROM usage_by_model_day_calls") + # The only call with usage is the run's one call, keyed as a single call is. + assert call["call_id"] == str(correlation_id) + assert call["model_id"] == "glm-5.2" + assert float(call["cost_usd"]) == pytest.approx(_delegation_cost(db_path)) From 21ef659b403e238f3cdf3c0efbd5b760e37b19bb Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Tue, 6 Oct 2026 14:24:44 +0530 Subject: [PATCH 09/13] 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 --- .../ports/port_local_delegation_dispatch.py | 35 +++++++++++++------ .../test_local_dispatch_usage_by_model_day.py | 28 ++++++++++++++- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py index d73328f2e0..5413f7ac16 100644 --- a/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py +++ b/src/omnimarket/nodes/node_delegate_skill_orchestrator/ports/port_local_delegation_dispatch.py @@ -3445,21 +3445,34 @@ def _project_usage( """Fold each call into the usage-by-model-day tables (OMN-20006). One call per attempt that reached a provider, on the model that served - it, with its own tokens, cost and usage source: an escalated run's - spend is split per model, and an attempt whose cost was not measured is - never booked as measured because a later one was. A call that carried - no usage at all (refused before a provider answered) is not a usage - call; its banked cost is zero, so the per-model sum still equals the - run's cost. A single call keeps the run's correlation id as its key; - several take ``:`` in attempt order. + it, with its own cost and usage source: an escalated run's spend is + split per model, and an attempt whose cost was not measured is never + booked as measured because a later one was. A call that carried no + usage at all (refused before a provider answered) is not a usage call; + its banked cost is zero, so the per-model sum still equals the run's + cost. A single call keeps the run's correlation id as its key; several + take ``:`` in attempt order. + + Tokens follow the one definition ``onex metering`` reads: the deciding + attempt's (the last one banked, whose tokens the delegation row + records). Every earlier attempt's call carries 0 tokens, so the window's + usage tokens equal metering's token totals (AC1). The columns are NOT + NULL, so 0 here means "not counted under that definition", not a + measured zero. Counting escalated-from attempts' tokens belongs in the + metering summary, so the CLI and the page move together. Dated by the terminal's own emitted_at, so its UTC day is the delegation row's. Each call has its own guard: a failed usage write must neither break the response nor take the call-metrics row, or another attempt's usage, down with it. """ - calls = [usage for usage in attempt_usage if usage.carries_usage] - for ordinal, usage in enumerate(calls, start=1): + deciding = len(attempt_usage) - 1 + calls = [ + (index == deciding, usage) + for index, usage in enumerate(attempt_usage) + if usage.carries_usage + ] + for ordinal, (is_deciding, usage) in enumerate(calls, start=1): call_id = ( str(correlation_id) if len(calls) == 1 @@ -3470,8 +3483,8 @@ def _project_usage( call_id=call_id, model_name=usage.model_id, tenant_id=tenant_id, - prompt_tokens=usage.tokens_in, - completion_tokens=usage.tokens_out, + prompt_tokens=usage.tokens_in if is_deciding else 0, + completion_tokens=usage.tokens_out if is_deciding else 0, estimated_cost_usd=float(usage.cost_usd), usage_source=usage.usage_source, timestamp=emitted_at, diff --git a/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py index 26676b80a4..a134ec6905 100644 --- a/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py +++ b/tests/unit/nodes/node_delegate_skill_orchestrator/test_local_dispatch_usage_by_model_day.py @@ -18,6 +18,8 @@ ``llm_call_metrics`` write down with it; * an escalated run books every attempt's spend to the final model, so the per-model split is wrong although the run's total is right; +* an escalated run's usage tokens disagree with ``onex metering``, which + counts only the deciding attempt's tokens (AC1, one definition); * an attempt whose cost was not measured is booked as measured because a later attempt's was; * a call that never reached a provider (no tokens, no cost) is counted as an @@ -52,6 +54,7 @@ handler_llm_delegation_call, transport, ) +from omnimarket.projection.sqlite_metering_reader import read_metering_records from omnimarket.routing import delegation_backend_resolution from tests.unit.nodes.node_delegate_skill_orchestrator.test_local_dispatch_escalation_omn13849 import ( _dispatch as _ladder_dispatch, @@ -286,6 +289,24 @@ def _rollups_by_model(db_path: Path) -> dict[str, sqlite3.Row]: } +def _usage_tokens(db_path: Path) -> tuple[int, int]: + (row,) = _rows( + db_path, + "SELECT SUM(input_tokens) AS i, SUM(output_tokens) AS o " + "FROM usage_by_model_day", + ) + return int(row["i"]), int(row["o"]) + + +def _metering_tokens(db_path: Path) -> tuple[int, int]: + """Token totals as ``onex metering --json`` reads them (AC1).""" + records = read_metering_records(db_path=db_path) + return ( + sum(r.tokens_in or 0 for r in records), + sum(r.tokens_out or 0 for r in records), + ) + + def _delegation_cost(db_path: Path) -> float: (row,) = _rows(db_path, "SELECT cost_usd FROM delegation_events") return float(row["cost_usd"]) @@ -311,7 +332,8 @@ def test_an_escalated_run_books_each_attempt_to_its_own_model( (c["model_id"], c["input_tokens"], c["output_tokens"], c["usage_source"]) for c in calls ] == [ - ("Qwen3.6-35B-A3B", 11, 22, "measured"), + # Tokens follow metering's definition: only the deciding attempt's. + ("Qwen3.6-35B-A3B", 0, 0, "measured"), ("glm-5.2", 33, 44, "measured"), ] assert [float(c["cost_usd"]) for c in calls] == pytest.approx([0.001, 0.010]) @@ -331,6 +353,8 @@ def test_an_escalated_run_books_each_attempt_to_its_own_model( run_cost = result["cost_usd"] assert isinstance(run_cost, float) assert total == pytest.approx(run_cost) + # AC1 holds for an escalated run: the window's usage tokens are metering's. + assert _usage_tokens(db_path) == _metering_tokens(db_path) == (33, 44) def test_a_failed_run_on_the_full_ladder_books_three_models_and_its_total( @@ -349,6 +373,7 @@ def test_a_failed_run_on_the_full_ladder_books_three_models_and_its_total( assert sum(float(r["cost_usd"]) for r in rollups.values()) == pytest.approx( _delegation_cost(db_path) ) + assert _usage_tokens(db_path) == _metering_tokens(db_path) == (55, 66) def test_an_unmeasured_attempt_is_never_booked_as_measured( @@ -373,6 +398,7 @@ def test_an_unmeasured_attempt_is_never_booked_as_measured( assert sum(float(r["cost_usd"]) for r in rollups.values()) == pytest.approx( _delegation_cost(db_path) ) + assert _usage_tokens(db_path) == _metering_tokens(db_path) == (33, 44) def test_a_call_that_reached_no_provider_is_not_a_usage_call( From 6ab85bd05df9f13b8159db411a41625f2e7d1fde Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 7 Oct 2026 08:52:39 +0530 Subject: [PATCH 10/13] 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 --- pyproject.toml | 8 ++++++-- uv.lock | 56 +++++++++++++++++++++++++------------------------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 93a28d32e9..d163f66c06 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -204,7 +204,9 @@ dependencies = [ # premium counterfactual is None, so every savings row is BASELINE_UNRESOLVED. # 0.38.61 declares omnibase-compat==0.5.7 and omnibase-spi==0.23.5; the # core floor above admits it, so this bump cascades nothing. - "omnibase-infra>=0.38.66,<0.39.0", + # 0.38.67 is the first release vendoring the projection migrations this branch adds + # (omnibase_infra#4547, #4559, #4568); the Sibling Release Compatibility Gate refuses them below it. + "omnibase-infra>=0.38.67,<0.39.0", "click>=8.1.0", "pydantic>=2.12.5,<3.0.0", "pydantic-settings>=2.0.0,<3.0.0", @@ -1013,7 +1015,9 @@ override-dependencies = [ # premium counterfactual is None, so every savings row is BASELINE_UNRESOLVED. # 0.38.61 declares omnibase-compat==0.5.7 and omnibase-spi==0.23.5; the # core floor above admits it, so this bump cascades nothing. - "omnibase-infra>=0.38.66,<0.39.0", + # 0.38.67 is the first release vendoring the projection migrations this branch adds + # (omnibase_infra#4547, #4559, #4568); the Sibling Release Compatibility Gate refuses them below it. + "omnibase-infra>=0.38.67,<0.39.0", "omnibase-compat==0.5.7", ] diff --git a/uv.lock b/uv.lock index fc2a6c6555..1cac0ab1a1 100644 --- a/uv.lock +++ b/uv.lock @@ -12,7 +12,7 @@ resolution-markers = [ overrides = [ { name = "omnibase-compat", specifier = "==0.5.7" }, { name = "omnibase-core", specifier = ">=0.47.31,<0.48.0" }, - { name = "omnibase-infra", specifier = ">=0.38.66,<0.39.0" }, + { name = "omnibase-infra", specifier = ">=0.38.67,<0.39.0" }, { name = "omnibase-spi", specifier = ">=0.23.3,<0.24.0" }, ] @@ -225,9 +225,9 @@ name = "aiologic" version = "0.17.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "sniffio" }, - { name = "typing-extensions" }, - { name = "wrapt" }, + { name = "sniffio", marker = "python_full_version < '3.13'" }, + { name = "typing-extensions", marker = "python_full_version < '3.13'" }, + { name = "wrapt", marker = "python_full_version < '3.13'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/f1/7a/d51f2fde1e8ae8a83431f8e97b7a71e9358cdb1d4d2ce6be387fa44d68de/aiologic-0.17.1.tar.gz", hash = "sha256:2e1b93b9e88ced318c2a63ad7b382688f40cbfe40e3d42258d49dc9c5aea179d", size = 252354, upload-time = "2026-06-27T20:41:33.25Z" } wheels = [ @@ -1192,7 +1192,7 @@ name = "cuda-bindings" version = "13.4.1" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "cuda-pathfinder" }, + { name = "cuda-pathfinder", marker = "python_full_version < '3.15'" }, ] wheels = [ { url = "https://files.pythonhosted.org/packages/d6/d0/76d0e45d98bf4933bf48eac6bbeb17464684540f69edec41fc37c7a422b0/cuda_bindings-13.4.1-cp312-cp312-manylinux_2_24_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:84ee88862e2e6ac39a5434c061f7f4389fbefbc418487d0670c86601d517d038", size = 6479976, upload-time = "2026-09-10T01:16:54.622Z" }, @@ -1223,43 +1223,43 @@ wheels = [ [package.optional-dependencies] cublas = [ - { name = "nvidia-cublas", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-cuda-nvrtc", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cublas", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-cuda-nvrtc", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] cudart = [ - { name = "nvidia-cuda-runtime", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cuda-runtime", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] cufft = [ - { name = "nvidia-cufft", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-nvjitlink", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cufft", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-nvjitlink", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] cufile = [ - { name = "nvidia-cufile", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cufile", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" }, ] cupti = [ - { name = "nvidia-cuda-cupti", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cuda-cupti", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] curand = [ - { name = "nvidia-curand", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-curand", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] cusolver = [ - { name = "nvidia-cublas", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-cusolver", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-cusparse", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-nvjitlink", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cublas", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-cusolver", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-cusparse", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-nvjitlink", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] cusparse = [ - { name = "nvidia-cusparse", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, - { name = "nvidia-nvjitlink", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cusparse", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, + { name = "nvidia-nvjitlink", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] nvjitlink = [ - { name = "nvidia-nvjitlink", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-nvjitlink", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] nvrtc = [ - { name = "nvidia-cuda-nvrtc", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-cuda-nvrtc", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] nvtx = [ - { name = "nvidia-nvtx", marker = "platform_machine == 'aarch64' or platform_machine == 'x86_64'" }, + { name = "nvidia-nvtx", marker = "(platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux') or (platform_machine == 'AMD64' and sys_platform == 'win32')" }, ] [[package]] @@ -1267,8 +1267,8 @@ name = "culsans" version = "0.11.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "aiologic" }, - { name = "typing-extensions" }, + { name = "aiologic", marker = "python_full_version < '3.13'" }, + { name = "typing-extensions", marker = "python_full_version < '3.13'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/d9/e3/49afa1bc180e0d28008ec6bcdf82a4072d1c7a41032b5b759b60814ca4b0/culsans-0.11.0.tar.gz", hash = "sha256:0b43d0d05dce6106293d114c86e3fb4bfc63088cfe8ff08ed3fe36891447fe33", size = 107546, upload-time = "2025-12-31T23:15:38.196Z" } wheels = [ @@ -3067,7 +3067,7 @@ wheels = [ [[package]] name = "omnibase-infra" -version = "0.38.66" +version = "0.38.67" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "a2a-sdk" }, @@ -3122,9 +3122,9 @@ dependencies = [ { name = "uvicorn" }, { name = "watchdog" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/cc/6e/57d7141fd52c4d80639f415c28683a8dc4663e326d4d4380374422c5d790/omnibase_infra-0.38.66.tar.gz", hash = "sha256:ab4d3749f3aa428503742fa5b786dd1b47f2618dc157ac871a4f62870590915c", size = 21342913, upload-time = "2026-10-05T17:10:51.058Z" } +sdist = { url = "https://files.pythonhosted.org/packages/34/57/5c0c42bd8c109410ede835ef56e38b5d914e0605da71c740fd2d7b55294a/omnibase_infra-0.38.67.tar.gz", hash = "sha256:65970fe4937ab50073fac69ef20bd4e300545f4cdd3f1825f732512cdaefa5d6", size = 21284688, upload-time = "2026-10-07T03:15:59.087Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/89/fd/e9439702ca7d855d31123069df8f30012392a5d1545e6dc10ec7be5c62e7/omnibase_infra-0.38.66-py3-none-any.whl", hash = "sha256:89310645b9e8c230633fb0926b24d73fd7af586ec45fcfaa5b7f3685ed21eec8", size = 7477834, upload-time = "2026-10-05T17:10:48.101Z" }, + { url = "https://files.pythonhosted.org/packages/b0/94/5357342fcc04bfbfa8aabda65415ea336c0f4f5dfb31974963aabb6f2449/omnibase_infra-0.38.67-py3-none-any.whl", hash = "sha256:169137d12c77e142f7ff76ce0ea1341019e363dd7debeeaffbaf9d7e663c80a3", size = 7508389, upload-time = "2026-10-07T03:15:56.304Z" }, ] [[package]] @@ -3221,7 +3221,7 @@ requires-dist = [ { name = "jsonschema", specifier = ">=4.18.0" }, { name = "omnibase-compat", specifier = "==0.5.7" }, { name = "omnibase-core", specifier = ">=0.47.31,<0.48.0" }, - { name = "omnibase-infra", specifier = ">=0.38.66,<0.39.0" }, + { name = "omnibase-infra", specifier = ">=0.38.67,<0.39.0" }, { name = "omnibase-spi", specifier = ">=0.23.3,<0.24.0" }, { name = "omninode-memory", specifier = "==0.18.0" }, { name = "packaging", specifier = ">=24.0" }, From fff35fa669575764c071126447386a959a772fa5 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 7 Oct 2026 09:46:51 +0530 Subject: [PATCH 11/13] 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 --- scripts/ci/customer_clean_install_delegate_probe.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/ci/customer_clean_install_delegate_probe.py b/scripts/ci/customer_clean_install_delegate_probe.py index 861eed1db0..344addd622 100644 --- a/scripts/ci/customer_clean_install_delegate_probe.py +++ b/scripts/ci/customer_clean_install_delegate_probe.py @@ -285,7 +285,10 @@ def probe(onex: Path, *, timeout: int) -> list[str]: return failures refused = _run( - [str(onex), "delegate", PROMPT], env=env, cwd=work, timeout=timeout + [str(onex), "delegate", PROMPT, "--json"], + env=env, + cwd=work, + timeout=timeout, ) overlay = home / ".omninode" / "delegation" / "bifrost_overrides.yaml" refusal_text = refused.stdout + refused.stderr @@ -308,7 +311,10 @@ def probe(onex: Path, *, timeout: int) -> list[str]: overlay.parent.mkdir(parents=True, exist_ok=True) overlay.write_text(customer_overlay_yaml(model.port), encoding="utf-8") done = _run( - [str(onex), "delegate", PROMPT], env=env, cwd=work, timeout=timeout + [str(onex), "delegate", PROMPT, "--json"], + env=env, + cwd=work, + timeout=timeout, ) terminal = _terminal(done.stdout) if done.returncode != 0: @@ -338,7 +344,7 @@ def probe(onex: Path, *, timeout: int) -> list[str]: ) posts_before = len(model.chat_posts()) short = _run( - [str(onex), "delegate", TWO_WORD_PROMPT], + [str(onex), "delegate", TWO_WORD_PROMPT, "--json"], env=env, cwd=work, timeout=timeout, From 56781c0e8eab2b7916123e8cf38a8924e071e841 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 7 Oct 2026 10:03:04 +0530 Subject: [PATCH 12/13] 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 --- .github/workflows/ci-bus-overlay-parity.yml | 6 +++--- .github/workflows/ci.yml | 2 +- .github/workflows/contract-topic-closure.yml | 2 +- .github/workflows/dispatcher-route-coverage.yml | 2 +- .github/workflows/node-migration-vendor-parity-gate.yml | 8 ++++---- .github/workflows/subscriber-dispatcher-resolution.yml | 2 +- tests/ci/test_ci_bus_overlay_parity.py | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci-bus-overlay-parity.yml b/.github/workflows/ci-bus-overlay-parity.yml index fdfbac91aa..5d1c7a47d5 100644 --- a/.github/workflows/ci-bus-overlay-parity.yml +++ b/.github/workflows/ci-bus-overlay-parity.yml @@ -106,7 +106,7 @@ jobs: # The omnibase_infra release this repo's uv.lock pins : a merge # in omnibase_infra cannot turn this repo's check red. Bump it with the # omnibase-infra lock pin, after an integration run against the new tag. - ref: v0.38.66 + ref: v0.38.67 path: .ci-bus-consumer token: ${{ secrets.OMNI_GITHUB_TOKEN || github.token }} persist-credentials: false @@ -122,7 +122,7 @@ jobs: set -euo pipefail SHA="$(git -C .ci-bus-consumer rev-parse HEAD)" echo "sha=${SHA}" >> "$GITHUB_OUTPUT" - echo "::notice::Consumer model resolved at omnibase_infra v0.38.66 ${SHA}" + echo "::notice::Consumer model resolved at omnibase_infra v0.38.67 ${SHA}" - name: Assert the consumer checkout produced the model shell: bash @@ -157,7 +157,7 @@ jobs: uv run python scripts/ci/check_ci_bus_overlay_parity.py \ --overlay config/ci_bus_lanes.yaml \ --consumer-model .ci-bus-consumer/scripts/trigger_rebuild_on_merge.py \ - --consumer-ref "omnibase_infra v0.38.66 ${{ steps.consumer.outputs.sha }}" + --consumer-ref "omnibase_infra v0.38.67 ${{ steps.consumer.outputs.sha }}" - name: Pin the gate's own behaviour run: | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e70618f744..9894b3b2bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3021,7 +3021,7 @@ jobs: uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: repository: OmniNode-ai/omnibase_infra - ref: v0.38.66 + ref: v0.38.67 path: .sibling/omnibase_infra token: ${{ secrets.GITHUB_TOKEN }} - name: Set up Python diff --git a/.github/workflows/contract-topic-closure.yml b/.github/workflows/contract-topic-closure.yml index b80aaaf694..00d241cc98 100644 --- a/.github/workflows/contract-topic-closure.yml +++ b/.github/workflows/contract-topic-closure.yml @@ -112,7 +112,7 @@ jobs: repository: OmniNode-ai/omnibase_infra # dev, not main: main is release-synced, so a validator that has landed on the # integration branch would otherwise not be visible until the next release. - ref: v0.38.66 + ref: v0.38.67 path: omnibase_infra token: ${{ secrets.OMNI_GITHUB_TOKEN || github.token }} sparse-checkout: | diff --git a/.github/workflows/dispatcher-route-coverage.yml b/.github/workflows/dispatcher-route-coverage.yml index 553966faf3..c615270960 100644 --- a/.github/workflows/dispatcher-route-coverage.yml +++ b/.github/workflows/dispatcher-route-coverage.yml @@ -68,7 +68,7 @@ jobs: uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: repository: OmniNode-ai/omnibase_infra - ref: v0.38.66 + ref: v0.38.67 path: omnibase_infra token: ${{ secrets.OMNI_GITHUB_TOKEN || github.token }} # Sparse checkout: only the gate script is needed diff --git a/.github/workflows/node-migration-vendor-parity-gate.yml b/.github/workflows/node-migration-vendor-parity-gate.yml index a52ad3e813..df5245d79b 100644 --- a/.github/workflows/node-migration-vendor-parity-gate.yml +++ b/.github/workflows/node-migration-vendor-parity-gate.yml @@ -91,7 +91,7 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: repository: OmniNode-ai/omnibase_infra - ref: v0.38.66 + ref: v0.38.67 path: omnibase_infra-dest token: ${{ secrets.OMNI_GITHUB_TOKEN || github.token }} sparse-checkout: | @@ -125,13 +125,13 @@ jobs: dest_file="omnibase_infra-dest/docker/migrations/forward/nodes/${node_name}/${filename}" if [ ! -f "${dest_file}" ]; then - echo "::warning::${src_file} has no vendored counterpart at docker/migrations/forward/nodes/${node_name}/${filename} in the pinned omnibase_infra v0.38.66. This repo is ahead of its pin; omnibase_infra vendors it through its own consumer PR (scripts/sync-node-migrations.sh) and this repo bumps the pin after integration testing." >&2 + echo "::warning::${src_file} has no vendored counterpart at docker/migrations/forward/nodes/${node_name}/${filename} in the pinned omnibase_infra v0.38.67. This repo is ahead of its pin; omnibase_infra vendors it through its own consumer PR (scripts/sync-node-migrations.sh) and this repo bumps the pin after integration testing." >&2 ahead=1 continue fi if ! cmp -s "${src_file}" "${dest_file}"; then - echo "::warning::${src_file} differs from the vendored copy in the pinned omnibase_infra v0.38.66 (${dest_file}). This repo is ahead of its pin; omnibase_infra re-runs scripts/sync-node-migrations.sh through its own consumer PR." >&2 + echo "::warning::${src_file} differs from the vendored copy in the pinned omnibase_infra v0.38.67 (${dest_file}). This repo is ahead of its pin; omnibase_infra re-runs scripts/sync-node-migrations.sh through its own consumer PR." >&2 ahead=1 fi done @@ -141,4 +141,4 @@ jobs: exit 0 fi - echo "::notice::All changed node migrations are already vendored in the pinned omnibase_infra v0.38.66." + echo "::notice::All changed node migrations are already vendored in the pinned omnibase_infra v0.38.67." diff --git a/.github/workflows/subscriber-dispatcher-resolution.yml b/.github/workflows/subscriber-dispatcher-resolution.yml index 7f04aa703c..540cb176fd 100644 --- a/.github/workflows/subscriber-dispatcher-resolution.yml +++ b/.github/workflows/subscriber-dispatcher-resolution.yml @@ -69,7 +69,7 @@ jobs: repository: OmniNode-ai/omnibase_infra # dev, not main: main is release-synced, so a validator that has landed on the # integration branch would otherwise not be visible here until the next release. - ref: v0.38.66 + ref: v0.38.67 path: omnibase_infra token: ${{ secrets.OMNI_GITHUB_TOKEN || github.token }} sparse-checkout: | diff --git a/tests/ci/test_ci_bus_overlay_parity.py b/tests/ci/test_ci_bus_overlay_parity.py index e14f90797c..7d35aeb503 100644 --- a/tests/ci/test_ci_bus_overlay_parity.py +++ b/tests/ci/test_ci_bus_overlay_parity.py @@ -625,7 +625,7 @@ def test_workflow_checks_out_the_consumer_at_its_default_branch(self) -> None: assert checkouts, "no checkout of OmniNode-ai/omnibase_infra" with_block = checkouts[0]["with"] - assert with_block["ref"] == "v0.38.66", ( + assert with_block["ref"] == "v0.38.67", ( "the gate must read the omnibase_infra release this repo pins in " "uv.lock, never omnibase_infra's live branch" ) From f1501de6674cc664bf0406e0917427156047c769 Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Wed, 7 Oct 2026 13:00:56 +0530 Subject: [PATCH 13/13] 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 --- src/omnimarket/projection/sqlite_database.py | 10 +++--- ..._usage_by_model_day_store_step_omn20006.py | 32 +++++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/omnimarket/projection/sqlite_database.py b/src/omnimarket/projection/sqlite_database.py index 06fe72e064..242bb6ca7a 100644 --- a/src/omnimarket/projection/sqlite_database.py +++ b/src/omnimarket/projection/sqlite_database.py @@ -561,12 +561,9 @@ def _apply_store_steps(cls, conn: sqlite3.Connection, db_path: Path) -> None: (step, datetime.now(UTC).isoformat()), ) conn.commit() - except BaseException as exc: + except sqlite3.OperationalError as exc: conn.rollback() - if not ( - isinstance(exc, sqlite3.OperationalError) - and (exc.sqlite_errorcode & 0xFF) == sqlite3.SQLITE_READONLY - ): + if (exc.sqlite_errorcode & 0xFF) != sqlite3.SQLITE_READONLY: raise logger.warning( "%s is read-only and has not %s; reading it as it is", @@ -574,6 +571,9 @@ def _apply_store_steps(cls, conn: sqlite3.Connection, db_path: Path) -> None: ", nor ".join(lacking for _, _, lacking in pending[index:]), ) return + except BaseException: + conn.rollback() + raise @staticmethod def _relabel_usage_source(conn: sqlite3.Connection) -> None: diff --git a/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py b/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py index 2b5e461243..97213e5feb 100644 --- a/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py +++ b/tests/unit/projection/test_sqlite_usage_by_model_day_store_step_omn20006.py @@ -20,6 +20,8 @@ read still takes the write lock; * the columns are added but the step record fails, so a half-done step is committed and never retried as a whole; +* a failed step propagates without rolling back, so its write lock outlives + the error and the next writer finds the store locked; * a busy writable store is passed off as read-only and read without the columns; * two first opens of one store race and the second adds a column twice. @@ -243,6 +245,32 @@ def test_a_failed_step_record_leaves_the_columns_unadded(tmp_path: Path) -> None assert USAGE_STEP not in _steps(path) +def test_a_failed_step_releases_the_write_lock_before_it_propagates( + tmp_path: Path, +) -> None: + # The step runs under BEGIN IMMEDIATE. A failure that is not a read-only + # store propagates, but its transaction is rolled back first: while the + # error is still held (its traceback keeps the failing connection alive), + # another writer of the store must not find it locked. + path = tmp_path / "evidence.sqlite" + _store_written_before_the_change(path) + with closing(sqlite3.connect(path)) as conn: + conn.execute( + f"CREATE TRIGGER refuse_usage_step BEFORE INSERT ON {STEPS_TABLE} " + f"WHEN NEW.step = '{USAGE_STEP}' " + "BEGIN SELECT RAISE(ABORT, 'step record refused'); END" + ) + conn.commit() + + with pytest.raises(sqlite3.IntegrityError, match="step record refused") as held: + SqliteDatabaseAdapter(path).query("usage_by_model_day") + + with closing(sqlite3.connect(path, timeout=0)) as other: + other.execute("BEGIN IMMEDIATE") + other.rollback() + assert held.value is not None + + def test_a_locked_store_written_before_the_change_raises_rather_than_reading( tmp_path: Path, caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -264,13 +292,13 @@ def test_two_first_opens_together_add_each_column_once(tmp_path: Path) -> None: path = tmp_path / "evidence.sqlite" _store_written_before_the_change(path) barrier = threading.Barrier(2) - errors: list[BaseException] = [] + errors: list[Exception] = [] def open_store() -> None: barrier.wait() try: SqliteDatabaseAdapter(path).query("usage_by_model_day") - except BaseException as exc: + except Exception as exc: errors.append(exc) threads = [threading.Thread(target=open_store) for _ in range(2)]