Skip to content

feat(archive): accept control-plane snapshot tables and policy source_cursor - #64

Merged
outerlook merged 2 commits into
developfrom
feat/archive-control-plane-pr678
Jul 10, 2026
Merged

feat(archive): accept control-plane snapshot tables and policy source_cursor#64
outerlook merged 2 commits into
developfrom
feat/archive-control-plane-pr678

Conversation

@outerlook

@outerlook outerlook commented Jul 10, 2026

Copy link
Copy Markdown
Member

Companion to usherlabs/fiet-maker#678 — must merge and deploy before (or together with) it.

fiet-maker#678 starts emitting three row shapes the forwarder cannot accept today:

  • strategy_data.policy_evaluation_events rows stamped with a new source_cursor column (block:<n>:log:<i> replay provenance). Without the column, the JSONEachRow insert fails the whole per-table group for any batch containing a cursor-stamped row — taking down the existing policy-clock stream, not just the new data.
  • strategy_data.market_identity and strategy_data.symbol_mapping (FIET-905 control-plane snapshots), currently rejected as unsupported tables.

Changes

  • schema/clickhouse/strategy_data.sql: source_cursor column on policy_evaluation_events plus an idempotent ALTER TABLE … ADD COLUMN IF NOT EXISTSensureArchiveSchema applies it at startup, so existing deployments migrate on the next forwarder restart with no manual DDL. New CREATE TABLEs for market_identity (ORDER BY (canonical_core_pool_id, event_time_ms)) and symbol_mapping (ORDER BY (exchange, trading_pair, event_time_ms)), following the sibling tables' provenance block and partitioning.
  • services/archive-forwarder/types.ts: the two new tables added to SUPPORTED_TABLES.
  • Contract fixture synced byte-for-byte with fiet-maker#678's copy (policy row now carries source_cursor).
  • Tests: unit coverage for parsing/grouping the new shapes; ClickHouse integration coverage for the old-shape migration path (drop column → ensureArchiveSchema → column present) and for inserting + querying all three row shapes end to end.

Verification

Full bun test with the compose ClickHouse up: 416 pass / 0 fail (integration suite executed against a live ClickHouse, not skipped; focused run 6 pass / 0 fail). biome check clean on changed files.

Rollout

  1. Merge + deploy this forwarder — schema self-applies at startup; old emitters are unaffected (column defaulted, tables additive).
  2. Then merge fiet-maker#678.

Summary by CodeRabbit

  • New Features

    • Added provenance cursor support to policy evaluation event records.
    • Added market identity and symbol mapping history tables.
    • Archive processing now accepts and stores these new record types and associated metadata.
  • Bug Fixes

    • Improved schema migration handling for the new provenance and history fields.
  • Tests

    • Expanded coverage for archiving, routing, schema migrations, and data persistence.

…_cursor

- strategy_data.market_identity + strategy_data.symbol_mapping DDL and
  SUPPORTED_TABLES entries (FIET-905 emitter rows)
- source_cursor column on policy_evaluation_events with idempotent
  startup migration (ensureArchiveSchema applies ALTER ... IF NOT EXISTS)
- contract fixture synced byte-for-byte with the emitter repo
- unit + ClickHouse integration coverage incl. old-shape migration path
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ba0aa20d-b369-4030-abe9-34ca37af4299

📥 Commits

Reviewing files that changed from the base of the PR and between 445f7b5 and e7de48b.

📒 Files selected for processing (5)
  • schema/clickhouse/strategy_data.sql
  • services/archive-forwarder/types.ts
  • test/archive-forwarder.test.ts
  • test/clickhouse-schema.integration.test.ts
  • test/fixtures/archive_forwarder_envelope.json
📜 Recent review details
🧰 Additional context used
🪛 ast-grep (0.44.1)
test/clickhouse-schema.integration.test.ts

[error] 502-510: Avoid SQL injection
Context: client.query({
query: SELECT source_hash FROM strategy_data.${table} WHERE deployment_id = {deployment_id:String},
query_params: { deployment_id: TEST_DEPLOYMENT },
format: "JSONEachRow",
})
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-typescript)

🔇 Additional comments (6)
schema/clickhouse/strategy_data.sql (1)

36-47: LGTM!

Also applies to: 75-101, 102-126

services/archive-forwarder/types.ts (1)

32-33: LGTM!

test/archive-forwarder.test.ts (1)

142-176: LGTM!

Also applies to: 194-201, 214-223, 361-362

test/fixtures/archive_forwarder_envelope.json (1)

16-16: LGTM!

test/clickhouse-schema.integration.test.ts (2)

17-23: LGTM!

Also applies to: 44-45, 59-95


373-514: 📐 Maintainability & Code Quality

Leave these direct client calls as-is. tsconfig.json excludes **/*.test.ts, so tsc won’t flag this path, and the rest of this test file already uses client directly.

			> Likely an incorrect or invalid review comment.

Walkthrough

The ClickHouse strategy schema now stores policy provenance cursors and identity/routeability snapshots. Archive-forwarder support, schema initialization, fixtures, cleanup, migrations, and persistence tests are updated for the new tables and fields.

Changes

Strategy archive support

Layer / File(s) Summary
Strategy archive schema contracts
schema/clickhouse/strategy_data.sql
Adds source_cursor to policy evaluation events and defines market_identity and symbol_mapping snapshot tables.
Archive-forwarder table support
services/archive-forwarder/types.ts, test/archive-forwarder.test.ts, test/fixtures/archive_forwarder_envelope.json
Recognizes the new tables, validates and groups their rows, checks schema initialization, and updates the archive fixture with source_cursor.
ClickHouse migration and archival validation
test/clickhouse-schema.integration.test.ts
Adds guaranteed client access, cleanup for the new tables, migration coverage for source_cursor, and archival persistence assertions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ArchiveBatch
  participant ArchiveForwarder
  participant ClickHouse
  ArchiveBatch->>ArchiveForwarder: submit policy and snapshot rows
  ArchiveForwarder->>ClickHouse: archive grouped rows
  ClickHouse-->>ArchiveForwarder: persist source_cursor and source_hash
Loading

Possibly related PRs

Poem

I hopped through schemas, crisp and bright,
With cursors tucked in rows just right.
Identity maps joined the flow,
While hashes watched the snapshots grow.
The archive burrow now runs clean!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: accepting new control-plane snapshot tables and policy rows with source_cursor.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/archive-control-plane-pr678

Comment @coderabbitai help to get the list of available commands.

@outerlook
outerlook merged commit 8884e94 into develop Jul 10, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant