Repository navigation
fix(OMN-12886): bind SEA dashboard to canonical projections - #198
Merged
jonahgabriel merged 2 commits intoJun 10, 2026
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 1 hour, 26 minutes, and 11 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR replaces the hackathon-specific projection with a canonical node-generation-completed projection, updating topic constants and contract, Postgres/SQLite readers and tests, ControlPlane frontend normalization and queries, component registry/visibility, and SEA demo template/layout. ChangesSEA Projection Migration
Sequence Diagram(s)sequenceDiagram
participant ControlPlanePage
participant useProjectionQuery
participant PostgresProjectionReader
participant generation_events
participant normalizeSeaProjectionRow
participant allEvents
ControlPlanePage->>useProjectionQuery: query node-generation-completed
useProjectionQuery->>PostgresProjectionReader: readProjection(onex.evt.omnimarket.node-generation-completed.v1)
PostgresProjectionReader->>generation_events: SELECT * ORDER BY COALESCE(timestamp, created_at) DESC LIMIT 500
generation_events-->>PostgresProjectionReader: SeaProjectionRow[]
PostgresProjectionReader-->>useProjectionQuery: SeaProjectionRow[]
useProjectionQuery-->>ControlPlanePage: data: SeaProjectionRow[]
ControlPlanePage->>normalizeSeaProjectionRow: map & coerce rows
normalizeSeaProjectionRow-->>ControlPlanePage: PipelineEvent[] (filtered)
ControlPlanePage->>allEvents: merge normalized + localEvents, sort by timestamp
allEvents-->>ControlPlanePage: PipelineEvent[] newest-first
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jonahgabriel
force-pushed
the
jonah/omn-12886-bounded-dashboard-sea-projection
branch
from
June 10, 2026 07:08
751ffe8 to
b7b4fd1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/templates/sea-demo.ts (1)
44-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRepair logic is additive-only, so legacy SEA widgets remain visible after migration.
repairSeaDemoDashboardcurrently appends missing required widgets but never removes deprecated ones. Persisted SEA dashboards created before this change can still render old unsupported panels, which conflicts with the migration goal.Suggested fix
export function repairSeaDemoDashboard(dashboard: DashboardDefinition): DashboardDefinition { if (!isSeaDemoDashboard(dashboard)) return dashboard; - - const present = new Set(dashboard.layout.map((item) => item.componentName)); - const missing = seaDemoTemplate.layout.filter( - (item) => SEA_DEMO_REQUIRED_COMPONENTS.has(item.componentName) && !present.has(item.componentName), - ); - - if (dashboard.layout.length > 0 && missing.length === 0) return dashboard; - - return { - ...dashboard, - layout: [ - ...dashboard.layout, - ...missing.map((item) => ({ - ...item, - config: { ...item.config }, - })), - ], - }; + const byName = new Map(dashboard.layout.map((item) => [item.componentName, item])); + const normalizedLayout = seaDemoTemplate.layout + .filter((item) => SEA_DEMO_REQUIRED_COMPONENTS.has(item.componentName)) + .map((item) => { + const existing = byName.get(item.componentName); + return { + ...item, + config: { ...(existing?.config ?? item.config) }, + }; + }); + + return { + ...dashboard, + layout: normalizedLayout, + }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/templates/sea-demo.ts` around lines 44 - 55, The current repairSeaDemoDashboard implementation only appends missing widgets and never removes deprecated ones; update it so the returned dashboard.layout filters out unsupported legacy widgets before adding required ones: within repairSeaDemoDashboard, build a set of required widget identifiers (from the same source that computes missing), filter dashboard.layout to keep only widgets whose id (or type) is in the allowed/required set, then append any missing entries (the existing missing.map logic) and return the new dashboard object; reference dashboard.layout, missing, and the repairSeaDemoDashboard function when making the change.scripts/generate-registry.ts (1)
1502-1514:⚠️ Potential issue | 🟠 Major | ⚡ Quick winControl-plane projection schema is stale after topic migration.
Line 1514 now binds to
TOPICS.nodeGenerationCompleted, but Lines 1502-1513 still require old event-log fields (type,message). The canonical snapshot incontract.yamlnow exposes generation artifact columns, so this manifest contract is out of sync.Proposed fix
projectionSchema: { type: 'object', - required: ['id', 'type', 'timestamp', 'message'], + required: [ + 'id', + 'correlation_id', + 'task_description', + 'provider', + 'model_id', + 'contract_passed', + 'timestamp', + 'created_at', + ], properties: { id: { type: 'string' }, - type: { type: 'string', enum: ['request', 'validation', 'success', 'error'] }, + correlation_id: { type: 'string' }, + task_description: { type: 'string' }, + provider: { type: 'string' }, + model_id: { type: 'string' }, + contract_passed: { type: 'number' }, timestamp: { type: 'string', format: 'date-time' }, - source: { type: 'string' }, - message: { type: 'string' }, - correlationId: { type: 'string' }, + created_at: { type: 'string', format: 'date-time' }, + contract_yaml: { type: 'string' }, + handler_source: { type: 'string' }, }, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate-registry.ts` around lines 1502 - 1514, The projectionSchema in the manifest is stale: it still requires old event-log fields like `type` and `message` while the dataSource binds to `TOPICS.nodeGenerationCompleted`; update the `projectionSchema` object (the `projectionSchema` literal near `dataSources: [projectionSource(TOPICS.nodeGenerationCompleted)]`) so its `properties` and `required` arrays match the canonical snapshot defined in contract.yaml (replace/remove old fields such as `type` and `message` and add the new generation-artifact columns and their types/constraints), ensuring the schema mirrors the contract.yaml snapshot for nodeGenerationCompleted events.
🧹 Nitpick comments (1)
src/components/dashboard/control-plane/ControlPlanePage.tsx (1)
66-75: 💤 Low valueConsider clarifying the
rowType === 'success'fallback semantics.The fallback
return rowType === 'success'at Line 74 assumes that whencontract_passedis missing or unparseable,row.typereliably indicates success. However, for canonical generation rows from the backend projection,row.typemay not be present until after this normalization derives it (Lines 100-106). This creates a circular dependency where the type derivation depends on a fallback that assumes the type already exists.If
contract_passedis always present in the backend projection (as the contract suggests), this fallback will never fire. If it's optional, the fallback should document what it means when bothcontract_passedandrow.typeare missing.💡 Suggested clarification
function contractPassed(value: unknown, rowType: unknown): boolean { if (typeof value === 'boolean') return value; if (typeof value === 'number') return value === 1; if (typeof value === 'string') { const normalized = value.trim().toLowerCase(); if (['true', '1', 'success', 'passed'].includes(normalized)) return true; if (['false', '0', 'error', 'failed'].includes(normalized)) return false; } + // Fallback: treat rows with explicit type='success' as passed. + // For canonical generation rows, contract_passed should always be present; + // this fallback handles legacy or malformed rows. return rowType === 'success'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/dashboard/control-plane/ControlPlanePage.tsx` around lines 66 - 75, The fallback in contractPassed(value, rowType) currently returns rowType === 'success', which creates a circular dependency when callers derive row.type from contractPassed; change the fallback to a well-defined, non-dependent value (e.g., return false or null/undefined) and document that behavior so callers know how to handle missing/unparseable contract_passed; update the function signature/JSdoc to allow rowType to be optional and ensure the caller that computes row.type (the normalization code around Lines ~100–106) does not rely on row.type being prepopulated by this fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/postgres-projection-reader.ts`:
- Line 539: The SQL ordering in postgres-projection-reader.ts uses ORDER BY
COALESCE(timestamp, created_at) DESC which conflicts with the projection
contract declaring order_by: "created_at DESC"; either change the SQL in the
query that contains ORDER BY COALESCE(timestamp, created_at) DESC to ORDER BY
created_at DESC to match the contract, or if the COALESCE fallback is required,
update the projection contract's order_by to "COALESCE(timestamp, created_at)
DESC" so they match (refer to the ORDER BY clause in the relevant query and the
contract's order_by field).
- Around line 535-543: The query in the switch case handling
'onex.evt.omnimarket.node-generation-completed.v1' uses SELECT * and returns all
columns, which violates the contract's 14-column subset; update the SQL in that
case to SELECT only the explicit columns listed in the contract (id,
correlation_id, task_description, provider, model_id, endpoint_class,
attempt_count, total_latency_e2e_ms, contract_passed, cost_inference_usd,
contract_yaml, handler_source, timestamp, created_at) while keeping the ORDER BY
COALESCE(timestamp, created_at) DESC LIMIT 500, and ensure the returned rows
(res.rows) are typed/cast to the expected Row[] shape in
postgres-projection-reader.ts so only the intended fields are exposed.
In `@server/sqlite-projection-reader.ts`:
- Around line 271-279: The query in the case for
'onex.evt.omnimarket.node-generation-completed.v1' uses SELECT * which returns
all columns rather than the 14-column subset defined by the projection contract;
change the SQL in the db.prepare call inside the switch case to explicitly list
the 14 columns (in the same names and order the contract requires) from
generation_events instead of using *, keeping the ORDER BY COALESCE(timestamp,
created_at) DESC and LIMIT 500, and ensure the returned rows match the
projection field names expected by the caller.
- Line 276: The SQL ORDER BY in sqlite-projection-reader.ts currently uses ORDER
BY COALESCE(timestamp, created_at) DESC which conflicts with the contract's
declared ordering of order_by: "created_at DESC"; update the query to use ORDER
BY created_at DESC to match the contract (or, if you truly need fallback
ordering by timestamp, update the contract to document that behavior), and
ensure the change is applied where the query string or function that builds it
(the SQL in sqlite-projection-reader) is defined so tests and consumers see the
consistent ordering.
---
Outside diff comments:
In `@scripts/generate-registry.ts`:
- Around line 1502-1514: The projectionSchema in the manifest is stale: it still
requires old event-log fields like `type` and `message` while the dataSource
binds to `TOPICS.nodeGenerationCompleted`; update the `projectionSchema` object
(the `projectionSchema` literal near `dataSources:
[projectionSource(TOPICS.nodeGenerationCompleted)]`) so its `properties` and
`required` arrays match the canonical snapshot defined in contract.yaml
(replace/remove old fields such as `type` and `message` and add the new
generation-artifact columns and their types/constraints), ensuring the schema
mirrors the contract.yaml snapshot for nodeGenerationCompleted events.
In `@src/templates/sea-demo.ts`:
- Around line 44-55: The current repairSeaDemoDashboard implementation only
appends missing widgets and never removes deprecated ones; update it so the
returned dashboard.layout filters out unsupported legacy widgets before adding
required ones: within repairSeaDemoDashboard, build a set of required widget
identifiers (from the same source that computes missing), filter
dashboard.layout to keep only widgets whose id (or type) is in the
allowed/required set, then append any missing entries (the existing missing.map
logic) and return the new dashboard object; reference dashboard.layout, missing,
and the repairSeaDemoDashboard function when making the change.
---
Nitpick comments:
In `@src/components/dashboard/control-plane/ControlPlanePage.tsx`:
- Around line 66-75: The fallback in contractPassed(value, rowType) currently
returns rowType === 'success', which creates a circular dependency when callers
derive row.type from contractPassed; change the fallback to a well-defined,
non-dependent value (e.g., return false or null/undefined) and document that
behavior so callers know how to handle missing/unparseable contract_passed;
update the function signature/JSdoc to allow rowType to be optional and ensure
the caller that computes row.type (the normalization code around Lines ~100–106)
does not rely on row.type being prepopulated by this fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f92bb00-1791-4854-adc1-48302fdf6c63
📒 Files selected for processing (14)
contract.yamlscripts/generate-registry.tsserver/__tests__/postgres-projection-reader.test.tsserver/postgres-projection-reader.tsserver/sqlite-projection-reader.test.tsserver/sqlite-projection-reader.tsshared/types/palette-visibility.tsshared/types/topics.test.tsshared/types/topics.tssrc/components/dashboard/control-plane/ControlPlanePage.test.tsxsrc/components/dashboard/control-plane/ControlPlanePage.tsxsrc/registry/component-registry.jsonsrc/templates/sea-demo.tssrc/templates/templates.test.ts
💤 Files with no reviewable changes (1)
- src/templates/templates.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/dashboard/control-plane/ControlPlanePage.tsx (1)
63-65: ⚡ Quick winRemove redundant
optionalTextwrapper.The
optionalTextfunction simply delegates totextwithout adding any logic. This unnecessary indirection reduces clarity.♻️ Refactor: replace all `optionalText` calls with direct `text` calls
Remove the function definition:
-function optionalText(value: unknown): string | null { - return text(value); -} -Then replace all 11 call sites (lines 119–130):
- selectedProvider: optionalText(row.selectedProvider ?? row.provider), + selectedProvider: text(row.selectedProvider ?? row.provider),Apply the same substitution to
selectedModel,endpointRef,resolvedEndpoint,routingSource,projectionOwner,projectionReducerVersion,contractYaml,handlerSource,outputPayloadSha256,contractSha256, andhandlerSha256.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/dashboard/control-plane/ControlPlanePage.tsx` around lines 63 - 65, Remove the redundant optionalText wrapper and replace all its call sites with direct calls to text: delete the optionalText function and update every usage (including selectedModel, endpointRef, resolvedEndpoint, routingSource, projectionOwner, projectionReducerVersion, contractYaml, handlerSource, outputPayloadSha256, contractSha256, and handlerSha256) to call text(...) instead of optionalText(...); ensure imports/ typings remain correct after removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/dashboard/control-plane/ControlPlanePage.tsx`:
- Around line 63-65: Remove the redundant optionalText wrapper and replace all
its call sites with direct calls to text: delete the optionalText function and
update every usage (including selectedModel, endpointRef, resolvedEndpoint,
routingSource, projectionOwner, projectionReducerVersion, contractYaml,
handlerSource, outputPayloadSha256, contractSha256, and handlerSha256) to call
text(...) instead of optionalText(...); ensure imports/ typings remain correct
after removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ebf0c41-18c5-4ae0-b229-f6bd74e1f229
📒 Files selected for processing (14)
contract.yamlscripts/generate-registry.tsserver/__tests__/postgres-projection-reader.test.tsserver/postgres-projection-reader.tsserver/sqlite-projection-reader.test.tsserver/sqlite-projection-reader.tsshared/types/palette-visibility.tsshared/types/topics.test.tsshared/types/topics.tssrc/components/dashboard/control-plane/ControlPlanePage.test.tsxsrc/components/dashboard/control-plane/ControlPlanePage.tsxsrc/registry/component-registry.jsonsrc/templates/sea-demo.tssrc/templates/templates.test.ts
💤 Files with no reviewable changes (2)
- src/templates/templates.test.ts
- shared/types/topics.ts
✅ Files skipped from review due to trivial changes (1)
- shared/types/topics.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- server/sqlite-projection-reader.test.ts
- shared/types/palette-visibility.ts
- server/sqlite-projection-reader.ts
- server/tests/postgres-projection-reader.test.ts
- src/components/dashboard/control-plane/ControlPlanePage.test.tsx
- src/registry/component-registry.json
- contract.yaml
- server/postgres-projection-reader.ts
- src/templates/sea-demo.ts
- scripts/generate-registry.ts
jonahgabriel
added this pull request to the merge queue
Jun 10, 2026
jonahgabriel
deleted the
jonah/omn-12886-bounded-dashboard-sea-projection
branch
June 10, 2026 07:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onex.evt.omnimarket.node-generation-completed.v1dataVerification
npm run checknpm run lintnpm run test:run -- src/components/dashboard/control-plane/ControlPlanePage.test.tsx src/templates/templates.test.ts shared/types/topics.test.ts server/sqlite-projection-reader.test.ts server/__tests__/postgres-projection-reader.test.ts tests/vite-middleware/proxy-config.test.tsOMNIBASE_CORE_PATH=/Users/jonah/Code/omni_home/omnibase_coreStability Closure Notes
devwas not mutated outside normal PR flow; prod remains blocked.Summary by CodeRabbit
New Features
Refactor