Skip to content

fix(OMN-19971): a declared key column the relation lacks never reaches the latest-per-key read - #3224

Merged
Patel230 merged 4 commits into
devfrom
lakshman/OMN-19971-snapshot-grain-virtual-key
Oct 2, 2026
Merged

Patel230 merged 4 commits into
devfrom
lakshman/OMN-19971-snapshot-grain-virtual-key

Conversation

@Patel230

@Patel230 Patel230 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Six snapshot exposures answer 503 projection_column_missing on the .201 dev lane since omnimarket#3208: delegation.summary, delegation.model-routing, delegation.quality-gate, delegation.token-usage, delegation.savings and cost.savings-overview. On 10-01 all six answered 200 and matched their writers. That is the Overview page's delegation and savings data. Found in the OMN-19971 readback of 2026-10-02 06:01Z (ticket comment bee62174); it also re-fires the catalogue check that no exposure marked ok answers 503.

Cause. #3208 added a latest-row-per-key read that selects, orders and compares on every declared key_columns entry. These six declare key_columns: [snapshot_grain, tenant_id]. snapshot_grain is the constant half of the bus compaction key, minted by the republish query, and deliberately not a column of their views (node_projection_delegation/contract.yaml, the snapshot_grain comment). Postgres raises UndefinedColumnError, and the reader maps it to 503.

What changed

  • src/omnimarket/projection/table_reader.py:
    • TableRowSource reads the relation's live column names once per relation from pg_attribute, with the relation as a bound value (to_regclass($1)), cached beside the cursor-type cache. It reads them only for an exposure that has key columns and a recency column.
    • New physical_key_columns(): the declared key columns the relation carries, in declared order. build_window_query takes relation_columns and keys the latest-row-per-key read on those. With none left, the per-key read is skipped.
    • A relation that does not resolve gives no columns, so every declared key is kept and the read itself still names the missing table.
  • No contract changes: dropping snapshot_grain from key_columns would break bus compaction (one live record per tenant per aggregate).
  • Version 0.4.279 (pyproject.toml, and only the omnimarket version line in uv.lock).

How it was verified

Failure modes written before the implementation, each with a test:

  • F1: a virtual key column reaches the SQL.
  • F2: a key column the relation does carry is dropped, collapsing distinct keys (the consumer-flow defect fix(OMN-20327): fix projection API cursor walks and lane-health contract #3208 fixed).
  • F3: every key column is virtual and the per-key read runs with no keys.
  • F4: a declared non-key column the relation lacks stops being refused by name.
  • F5: the column lookup is interpolated or repeated, or a missing relation stops being refused by name.

Tests:

  • tests/unit/projection/test_omn19971_virtual_key_columns.py (4): SQL shape for F1–F3, and the bound, cached lookup (F5).
  • tests/integration/projection/test_omn19971_virtual_key_real_postgres.py (5), on a real disposable PostgreSQL: a per-tenant view with no snapshot_grain (F1), consumer-flow's shape with one busy and two quiet keys (F2), a virtual-only key (F3), a missing non-key column (F4), a missing relation (F5).
  • Red before: with dev's table_reader.py (750808959), F1 and F3 fail and F2, F4 and F5 pass. The last three are guard rails, and they pass before and after.
  • Green after: 9 passed; tests/unit/projection, tests/unit/nodes/node_projection_read_effect, tests/integration/nodes/node_projection_read_effect and the new files give 443 passed, 5 skipped. mypy --strict on table_reader.py, ruff check and ruff format --check are clean.

Lab first, on the lakshman lane on .201 (read-only, 2026-10-02T07:41:16Z):

  • For each of the six exposures, build_window_query built the SQL from the real contracts. relation_columns came from the lab views' own pg_attribute, and the tenant was 820272f9-…. Each query ran in a READ ONLY transaction on omnibase-infra-lakshman-postgres.
  • Dev's SQL: 6 of 6 fail with ERROR: column "snapshot_grain" does not exist.
  • This branch's SQL: 6 of 6 return 1 row, equal to each view's row count for that tenant.
  • Not covered here: the read through a rebuilt lakshman-lane projection API, and the 69-exposure readback on the shared dev lane. The second needs this merged and the dev lane redeployed; it then re-proves that no exposure marked ok answers 503.

Failure paths

n/a: this change writes no verdict, receipt or report. It changes which columns one read keys on.

Open defects

none

Local gates

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

Not in this change

Overlap-Reviewed: #2965 — both touch pyproject.toml and uv.lock: #2965 raises the omnibase-core floor to 0.47.24 and removes its git-rev override; this changes only omnimarket's own version line (0.4.278 to 0.4.279). Different lines and different purposes, so each is needed; whichever merges second takes the next version.

Evidence-Ticket: OMN-19971
Evidence-Source: OCC#12295

…s the latest-per-key read

Six snapshot exposures (delegation summary, model-routing, quality-gate,
token-usage, savings, cost savings-overview) key on (snapshot_grain,
tenant_id). snapshot_grain is the constant half of the bus compaction key
and deliberately not a column of their views. The latest-row-per-key read
from OMN-20327 selected every declared key column, so all six answered 503
projection_column_missing on the dev lane (readback 2026-10-02 06:01Z).

TableRowSource now reads the relation's live columns once (bound
to_regclass($1), cached) and keys the read only on declared key columns
the relation carries. With none left, the per-key read is skipped. The
contracts stay as they are, so bus compaction is unchanged.

Tests: unit (SQL shape, bound and cached lookup) and real Postgres. On
dev's reader the two snapshot cases fail; consumer-flow's every-key walk,
a missing non-key column and a missing relation behave the same before
and after. Version 0.4.279.
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

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

No architectural violations found.

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

onexbot-occ-writer Bot pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Oct 2, 2026
…3224

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 2b392042c2f3149348d417b0ef276cd2037541f4.
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

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


Gate semantics

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

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

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

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

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

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

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

* evidence(OMN-19971): self-bind OCC#12295 + rebind contract_sha256

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

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

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

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
Co-authored-by: onexbot-occ-writer[bot] <307849072+onexbot-occ-writer[bot]@users.noreply.github.com>
…orted name

CodeQL flagged the module-level _FIXTURES tuple on #3224 as an unused
global. The dsn fixture now requests local_postgres through the imported
object's own __name__, which uses the import without a suppression
comment or a placeholder global. 5 passed on a real PostgreSQL.
Resolves the version conflict: dev moved to 0.4.281, so this branch takes
dev's pyproject.toml and uv.lock and bumps omnimarket to 0.4.282 (only the
two version lines differ from dev). Unit projection tests: 400 passed, 5
skipped; tests/integration/projection/test_omn19971_virtual_key_real_postgres.py:
5 passed.
@Patel230
Patel230 merged commit efc25b3 into dev Oct 2, 2026
148 checks passed
@Patel230
Patel230 deleted the lakshman/OMN-19971-snapshot-grain-virtual-key branch October 2, 2026 16:57
@Patel230

Patel230 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

The CodeQL note on tests/integration/projection/test_omn19971_virtual_key_real_postgres.py (unused _FIXTURES, 2026-10-02 08:08Z) was fixed before merge in db029a5c9: the test now requests the disposable PostgreSQL fixture by name, so the unused global is gone.

Merged as efc25b3 on 2026-10-02. It fixes the 503 projection_column_missing on the six snapshot exposures that declare snapshot_grain; every test split and CodeQL passed on the final head.

Left on OMN-19971: AC1 is still not met on the dev lane because of two other problems read on 2026-10-03 (intermittent 503 timeouts on three exposures, and a decisions window serving old rows).

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants