Skip to content

High-value brainstorm + first two implementations: paper-session TCA and data replacement-cost meter - #2078

Merged
rodoHasArrived merged 14 commits into
mainfrom
claude/prioritize-high-value-code-88hyfu
Jul 1, 2026
Merged

High-value brainstorm + first two implementations: paper-session TCA and data replacement-cost meter#2078
rodoHasArrived merged 14 commits into
mainfrom
claude/prioritize-high-value-code-88hyfu

Conversation

@rodoHasArrived

@rodoHasArrived rodoHasArrived commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Summary

A market-researched brainstorm document prioritizing the ten highest-value implementable features, plus working implementations of four of them, with review findings addressed along the way.

Brainstorm (docs):

Idea #1 — paper/live session TCA:

  • SessionTcaReporter (Meridian.Execution): commission attribution, per-symbol VWAP, median-based outlier detection, execution-quality stats. Cumulative per-order fill sequences (IB-style running totals) are normalized using order quantity and order-history executed quantity. GET /api/execution/sessions/{sessionId}/tca.

Idea #10 — data replacement-cost meter:

  • DataReplacementCostEstimator (Meridian.Storage): conservative estimate (lower of per-symbol-day and per-gigabyte bases) against a configurable rate card. GET /api/workstation/data/replacement-cost plus an "Est. Replacement Cost" metric card on the Data workspace.

Idea #4 — IB Flex statement connector:

  • IbFlexBrokerStatementService parses Flex Query XML (Trades, OpenPositions, CashTransactions) into canonical statement rows (fee-like cash transactions keep fee semantics); RoutingBrokerStatementService dispatches by broker alias or .xml extension; ib-flex-v1 mapping profile registered. Statement-run validation, checkpointed ingestion, and case intake all handle Flex sources end-to-end, including generic-broker .xml files; imports with no supported rows fail loudly.

Idea #5 — DuckDB SQL query workbench:

  • DuckDbQueryService (Meridian.Storage, via DuckDB.NET.Data.Full under central package management): read-only SQL over the local Parquet/JSONL store with a meridian_files catalog discovery view, session memory/thread limits, statement guard (single SELECT-family statement, write/config keyword blocklist), row cap, and timeout. POST /api/workstation/data/query returns guard/SQL errors in-band.
  • Data screen "SQL query" panel: editor, run button, row-count/elapsed status, inline errors, results table, with a dedicated view-model hook.

Options consumed by workstation endpoints (DataReplacementCost, DataQuery) bind in the shared workstation service registration so every endpoint host honors operator configuration.

Reason

Requested brainstorm of the highest-value code to work on next, followed by requests to implement the ideas in the recommended sequence.

Testing performed

  • bash scripts/ci.sh completed successfully
  • Relevant unit tests were added or updated
  • Relevant integration tests were added or updated
  • GitHub Actions quality-gate passed

New tests: SessionTcaReporterTests (14), DataReplacementCostEstimatorTests (8), IbFlexStatementServiceTests (26), SqlStatementGuardTests + DuckDbQueryServiceTests (36), query-panel vitest (4). Affected suites pass: workstation endpoints + execution (523), reconciliation + statement import + storage + endpoints (641), dashboard vitest suite green, full solution builds, dotnet format whitespace --verify-no-changes clean. Full quality-gate runs on this PR.

Safety review

  • This pull request targets main
  • No direct push to main was performed
  • No tests were disabled or bypassed
  • No secrets or credentials were committed
  • No unrelated changes were included

Governance changes

Check every governance file modified:

  • No governance files changed
  • .github/workflows/**
  • .github/CODEOWNERS
  • .github/pull_request_template.md
  • AGENTS.md
  • scripts/ci.sh

🤖 Generated with Claude Code

https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

Adds a prioritized, market-evidence-backed brainstorm of the ten
highest-value implementable features, grounded in the roadmap registry
(W6/W7 planned lanes), the competitive-landscape reference, and July
2026 market research on fund-ops platforms, TCA, open-source quant
stacks, and market-data pricing. Records the session in the brainstorm
history ledger.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds a new entry to the brainstorm history and introduces a comprehensive market-researched prioritization document outlining ten high-value feature ideas for the Meridian platform. The review feedback is highly constructive, pointing out an architectural inconsistency regarding the Alpaca adapter's interface reference (suggesting IExecutionGateway instead of IOrderGateway) and recommending minor markdown formatting improvements to ensure code identifiers are consistently wrapped in backticks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

{"session_date": "2026-03-27", "mode": "Domain-Focused", "themes": ["Script-to-strategy promotion wizard", "Script notebook mode (multi-cell)", "Script library / import system", "Scheduled scripts and Alert API", "QuantScript parameter grid search API", "Live streaming data in scripts (v2)", "L3 x QuantScript bridge (L3.Simulate())", "Calibration quality report card", "Toxic flow tagging on fill tape", "L3 simulation replay visualizer", "Options queue model extension", "Symbol-partitioned collection (sharding)", "Leader-elected coordinator instance", "Cross-instance deduplication ledger (IDedupStore)", "Distributed backfill work queue", "Cluster health dashboard panel", "Distributed grid search (Track G x I)", "Cross-symbol correlation regime signals"], "ideas_count": 18, "track": "G-H-I", "notes": "First round of Track G-I brainstorm. Defined L3.Simulate() as highest-leverage cross-track bridge. IDedupStore as foundational Track I seam. Recommended sequencing: CalibrationReportCard -> ScriptLibrary -> L3Bridge -> GridSearch -> IDedupStore -> LeaderElection -> Sharding -> DistributedBackfill."}
{"session_date": "2026-03-27", "mode": "Domain-Focused", "themes": ["Script debugger (step/watch/breakpoint)", "Script run history and result diffing", "Security Master access in QuantScript (Securities global)", "Script output export (PDF/Parquet/Excel/HTML)", "Script expression REPL", "Venue-specific L3 calibration profiles (VenueCalibrationPriors)", "TCA report: execution cost attribution", "Multi-day queue state continuity (carry-state)", "Dark pool / hidden liquidity estimator", "Zero-downtime rolling upgrade orchestration", "Geo-distributed collection by exchange timezone", "Split-brain detection and recovery"], "ideas_count": 12, "track": "G-H-I", "notes": "Second round of Track G-I brainstorm. TCA Report identified as highest-leverage institutional positioning item. VenueCalibrationPriors and SplitBrainDetector identified as platform bets. Script REPL flagged as surprise high-leverage small-effort item. Sequencing: REPL + SecurityMaster -> TCA Report -> RunHistory + VenueCalibration -> SplitBrain -> RollingUpgrade + DarkPool + GeoDistributed + Debugger + MultiDayContinuity."}
{"session_date": "2026-06-25", "mode": "Domain-Focused", "themes": ["Provider Setup Wizard with Live Certification", "Hot-Reload Provider Re-Registration", "Credential Vault with Encrypted Persistence", "Runtime Failover Chain Editor", "Per-Provider Partial Config Mutation API", "Ingestion Session Continuity on Restart", "Provider Capability Browser in Data Workspace"], "ideas_count": 7, "document_updated": null, "notes": "Fresh domain covering runtime provider lifecycle management — an unexplored area in all prior sessions. Platform bets: Hot-Reload Re-Registration (ConfigWatcher already exists, ProviderRegistry.RegisterStreamingFactory already supports replace) and Per-Provider Partial Config Mutation (ConfigStore.PatchAsync primitive). Recommended sequence: Capability Browser (S) -> Partial Config API (M) -> Hot-Reload (M) -> Failover Editor (S) -> Setup Wizard (M) -> Credential Vault (M) -> Session Continuity (L)."}
{"session_date": "2026-07-01", "mode": "Competitive / Open Exploration", "themes": ["live/paper TCA best-execution report pack", "Alpaca live broker adapter (IOrderGateway)", "reconciliation break-resolution agent (MCP + CoS gates)", "custodian/broker statement connector library (IB Flex, OFX/CSV profiles)", "DuckDB analytics workbench + /api/query", "Python meridian client package (promoted)", "W6 backtesting studio evidence loop (manifest + run comparison)", "LEAN/NautilusTrader data-feed bridge", "governed investor report delivery portal-lite", "data-cost savings meter"], "ideas_count": 10, "document_updated": "docs/plans/high-value-code-brainstorm-2026-07.md", "notes": "Market-researched prioritization session. Signals: TCA now mandatory infrastructure (~$2.8B market), Arcesium productizing Reconciliation Agent, LEAN BYO-data pain, Databento/Polygon pricing validates self-hosted economics. Sequencing: cost meter + TCA + IB Flex connector first; DuckDB -> Python package; Alpaca adapter feeding W7; W6 studio slice; recon agent after connector coverage."}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To align with the architecture described in the implementation plan, the Alpaca live broker adapter should be associated with IExecutionGateway (the broker adapter contract) rather than IOrderGateway (the strategy-facing execution contract). Consider updating the theme name in the ledger to reflect this distinction.

{"session_date": "2026-07-01", "mode": "Competitive / Open Exploration", "themes": ["live/paper TCA best-execution report pack", "Alpaca live broker adapter (IExecutionGateway)", "reconciliation break-resolution agent (MCP + CoS gates)", "custodian/broker statement connector library (IB Flex, OFX/CSV profiles)", "DuckDB analytics workbench + /api/query", "Python meridian client package (promoted)", "W6 backtesting studio evidence loop (manifest + run comparison)", "LEAN/NautilusTrader data-feed bridge", "governed investor report delivery portal-lite", "data-cost savings meter"], "ideas_count": 10, "document_updated": "docs/plans/high-value-code-brainstorm-2026-07.md", "notes": "Market-researched prioritization session. Signals: TCA now mandatory infrastructure (~$2.8B market), Arcesium productizing Reconciliation Agent, LEAN BYO-data pain, Databento/Polygon pricing validates self-hosted economics. Sequencing: cost meter + TCA + IB Flex connector first; DuckDB -> Python package; Alpaca adapter feeding W7; W6 studio slice; recon agent after connector coverage."}

Comment on lines +57 to +61
| 2 | Alpaca live broker adapter behind `IOrderGateway` | M | H, I | High | — |
| 3 | Reconciliation break-resolution agent (MCP + CoS gates) | M | I | High | — |
| 4 | Custodian/broker statement connector library | M | I, H | High | — |
| 5 | DuckDB analytics workbench in the Data workspace | M | Q, H | High | — |
| 6 | Python `meridian` client package | M | H, Q | High | 5 (optional) |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the rest of the document, code identifiers in the table should be wrapped in backticks. Specifically:\n- IOrderGateway on line 57\n- meridian on line 61\n\nSuggested table rows:\nmarkdown\n| 2 | Alpaca live broker adapter behind `IOrderGateway` | M | H, I | High | — |\n...\n| 6 | Python `meridian` client package | M | H, Q | High | 5 (optional) |\n

**Why now:** feeds W7 live-readiness directly; institutional differentiator no open-source
competitor has; models already exist and are tested on the backtest side.

### 2. Alpaca Live Broker Adapter Behind `IOrderGateway`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other section headers and code references, wrap IOrderGateway in backticks.\n\nSuggested header:\nmarkdown\n### 2. Alpaca Live Broker Adapter Behind `IOrderGateway`\n

Regenerates the generated status docs (doc health dashboard, coverage
report, API contract coverage, TODO index, example validation,
repository structure) so the regenerate-docs check passes with the new
brainstorm document included. Also renames the Alpaca adapter idea to
reference IExecutionGateway, the broker adapter contract it would
implement, rather than the strategy-facing IOrderGateway, per review
feedback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC
@rodoHasArrived
rodoHasArrived marked this pull request as ready for review July 1, 2026 20:03
Copilot AI review requested due to automatic review settings July 1, 2026 20:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 611210d3cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +107 to +108
`PaperTradingGateway` (`src/Meridian.Execution/Adapters/PaperTradingGateway.cs`) is still the only
live-capable adapter. W7-LIVE-001 — live-readiness governance — has nothing real to govern until at

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retarget the Alpaca item to existing live-gateway gaps

This prioritization is based on a stale premise: AlpacaBrokerageGateway already exists and implements IBrokerageGateway, which extends IExecutionGateway (src/Meridian.Infrastructure/Adapters/Alpaca/AlpacaBrokerageGateway.cs:43, src/Meridian.Execution.Sdk/IBrokerageGateway.cs:9), and the hosted app registers it under the alpaca key. As written, the roadmap would send someone to build a duplicate “first adapter” instead of hardening the real remaining gaps, such as promotion wiring, live-readiness evidence, broker-state reconciliation, or governance UX.

Useful? React with 👍 / 👎.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This is a documentation-only PR that adds a market-researched, prioritized brainstorm of the ten highest-value implementable features (docs/plans/high-value-code-brainstorm-2026-07.md), records the session in the brainstorm skill's history ledger, and regenerates the repo's generated status/structure docs so the regenerate-docs gate stays clean. It fits the repository's evidence-led planning workflow and ties its ideas to the planned W6 (Backtesting Studio) and W7 (live-readiness) roadmap lanes. No runtime behavior changes.

Changes:

  • Adds a 376-line brainstorm document with a market-research summary, ten codebase-anchored ideas, a synthesis/sequencing plan, and cited sources.
  • Appends a 2026-07-01 session entry to brainstorm-history.jsonl (10 themes, matching ideas_count).
  • Regenerates six generated status/structure docs; note that most of the regenerated diff is pre-existing drift from source files already on the branch (providers, tenancy, migrations, dashboard components/screens, screenshots), which the docs gate forces to be captured wholesale rather than being unique to this doc.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/plans/high-value-code-brainstorm-2026-07.md New long-form brainstorm doc; placed in docs/plans/, which the folder README's Migration Rule 1 discourages (results in an orphaned file).
.claude/skills/meridian-brainstorm/brainstorm-history.jsonl Appends a well-formed session ledger entry per the skill's COMPLETE step.
docs/status/doc-health-dashboard.md / .json Increment doc/line/orphan counts and register the new doc; internally consistent.
docs/status/coverage-report.md Refreshed documented/undocumented counts and shifted line references.
docs/status/api-contract-coverage-dashboard.md / .json Updated endpoint source line numbers reflecting current source.
docs/status/example-validation.md Scanned-file count 418 → 419 for the new doc.
docs/status/TODO.md Line-number shifts for existing dashboard TODO markers; total unchanged (200).
docs/generated/repository-structure.md Regenerated tree now including the new doc plus accumulated repo tree drift.

@@ -0,0 +1,376 @@
# High-Value Code Brainstorm — Market-Researched Prioritization (2026-07)
Implements the two 'Now'-lane items from the high-value brainstorm:

- SessionTcaReporter (Meridian.Execution): TCA over paper/live session
  fill tapes — commission attribution, per-symbol VWAP breakdowns,
  median-based outlier detection, and execution-quality stats
  (time-to-fill, notional-weighted limit-price improvement) — exposed
  at GET /api/execution/sessions/{sessionId}/tca.
- DataReplacementCostEstimator (Meridian.Storage): conservative
  replacement-cost estimate of the local market-data store priced
  against a configurable vendor rate card (lower of per-symbol-day and
  per-gigabyte bases), exposed at
  GET /api/workstation/data/replacement-cost and as an
  'Est. Replacement Cost' metric card on the Data workspace payload.
- 18 new xUnit tests (SessionTcaReporterTests,
  DataReplacementCostEstimatorTests); workstation endpoint and
  execution suites pass (523 tests).

Also addresses review feedback: relocates the brainstorm doc to
docs/product/ per the docs/plans migration rule and links it from the
product index; retargets brainstorm idea #2 to hardening the existing
AlpacaBrokerageGateway (promotion wiring, broker-state reconciliation,
live-readiness evidence) instead of building a duplicate adapter; and
regenerates the generated status docs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC
@rodoHasArrived rodoHasArrived changed the title docs: market-researched high-value code brainstorm (2026-07) High-value brainstorm + first two implementations: paper-session TCA and data replacement-cost meter Jul 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 826a4d593a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


foreach (var fill in fills)
{
var notional = fill.FilledQuantity * fill.FillPrice!.Value;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize cumulative fill quantities before summing TCA

When a session contains successive partial/final reports whose FilledQuantity is cumulative rather than incremental, this calculation treats each snapshot as a new fill and overstates notional, commissions per notional, VWAP, and limit-price improvement. I checked the existing Interactive Brokers adapter path, which builds fill reports from update.CumulativeQuantity, so a 40-share partial followed by a 100-share final report would be counted as 140 shares in the new TCA report instead of the 100 shares actually executed.

Useful? React with 👍 / 👎.

Comment on lines +4177 to +4179
var options = context.RequestServices
.GetService<IOptionsMonitor<DataReplacementCostOptions>>()?.CurrentValue
?? new DataReplacementCostOptions();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind replacement-cost options before reading them

I searched the repo and DataReplacementCostOptions is only referenced by the estimator, this endpoint, and tests, so there is no AddOptions<DataReplacementCostOptions>().BindConfiguration(...) registration. In deployments that set DataReplacementCost:Enabled=false or update the rate card, this path still falls back to the hard-coded defaults, causing the workstation metric and endpoint to report replacement costs that operators tried to disable or change.

Useful? React with 👍 / 👎.

Addresses two review findings:

- Registers DataReplacementCostOptions with BindConfiguration in the
  UiServer host so DataReplacementCost:Enabled=false and rate-card
  overrides in configuration take effect instead of silently falling
  back to compiled defaults.
- SessionTcaReporter now detects per-order cumulative quantity
  sequences (running totals, as emitted by the IB brokerage gateway)
  and converts them to incremental fills before aggregation, so a
  40-share partial followed by a 100-share final report counts as 100
  executed shares rather than 140. Incremental tapes are unaffected;
  sequences with unknown order quantity are left untouched. Adds three
  regression tests covering cumulative, incremental-complete, and
  incremental-working sequences.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b16263d03a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

sum += report.FilledQuantity;
}

return sum > orderQuantity;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Normalize cumulative partials before the final fill

Fresh evidence in this revision is that IsCumulativeSequence only normalizes monotonic reports when their summed FilledQuantity exceeds the full OrderQuantity. For live IB fills, IBBrokerageGateway writes update.CumulativeQuantity into ExecutionReport.FilledQuantity (src/Meridian.Infrastructure/Adapters/InteractiveBrokers/IBBrokerageGateway.cs:679), so an in-progress 100-share order with cumulative partials 10 then 20 has sum == 30 <= 100 and stays unnormalized; the TCA endpoint then reports 30 shares/notional until the final fill arrives, overstating exactly the live session costs operators would monitor mid-execution. Consider using the latest order-history filled quantity or explicit source semantics instead of only the full order quantity.

Useful? React with 👍 / 👎.

…zation

Implements the custodian/broker statement connector lane from the
brainstorm (idea #4), starting with Interactive Brokers Flex Query XML:

- IbFlexBrokerStatementService parses Flex reports (Trades,
  OpenPositions, CashTransactions) into canonical statement rows with
  the same duplicate-key, checksum, and persistence flow as the CSV
  path, so matching and case intake are source-agnostic. XML loading
  prohibits DTD processing.
- RoutingBrokerStatementService dispatches imports by broker alias
  (ib-flex, ibflex, ibkr, interactive-brokers) or .xml extension,
  keeping all existing CSV flows unchanged; registered as the
  IBrokerStatementService implementation.
- Registers an 'ib-flex-v1' statement mapping profile (Flex attribute
  names, IB transaction-code mappings) resolvable by source kind.
- 19 connector tests covering validation, section mapping, Flex date
  formats, duplicate detection, routing, matching flow, and profile
  registration.

Also addresses review feedback on SessionTcaReporter: cumulative
sequences on still-working orders (partials summing below the order
quantity) are now detected via the order history's executed quantity,
so mid-session TCA no longer overstates in-progress IB orders. Adds a
regression test and updates the storage feature registration test for
the routing service.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a62d58d4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

new CsvBrokerStatementService(sp.GetRequiredService<ICanonicalStatementStore>()));
services.TryAddSingleton<IbFlexBrokerStatementService>(sp =>
new IbFlexBrokerStatementService(sp.GetRequiredService<ICanonicalStatementStore>()));
services.TryAddSingleton<IBrokerStatementService>(sp => new RoutingBrokerStatementService(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Let statement-run validation accept IB Flex

When an operator creates a statement run with Broker = "ib-flex" (the alias this registration now routes to IbFlexBrokerStatementService), StatementRunWorkflowService.NormalizeAndValidateAsync calls the validation service before brokerStatementService.ImportAsync. That validator is still StatementReconciliationService, whose source-kind whitelist excludes ib-flex and whose generic broker path validates CSV headers, so the workstation/CLI import flow rejects Flex XML before the new router ever runs. Please extend the validation path or route it through the same Flex aliases/extensions as the importer.

Useful? React with 👍 / 👎.

…tion

The statement-run workflow validates through
StatementReconciliationService before the broker statement router runs,
and its source-kind whitelist rejected 'ib-flex' (and applied CSV
header checks to XML when a mapping profile was supplied), blocking
Flex imports end-to-end. Per review feedback:

- ValidateSourceAccess now recognizes the Flex aliases (ib-flex,
  ibflex, ibkr, interactive-brokers) and normalizes them to 'ib-flex'.
- Validation checks the Flex XML document shape (FlexQueryResponse
  root, DTD processing prohibited) instead of CSV headers.
- The checkpointed ingestion stage reads Flex reports into source-row
  references (one per Trade/OpenPosition/CashTransaction element with
  attribute snapshots) so row counts are real; canonical row
  construction stays with the Flex-aware broker statement importer.
- Case intake never routes Flex files through the canonical CSV path.

Adds an end-to-end StatementRunWorkflowService test importing a Flex
statement through validation, routing, matching, and case intake, plus
validation-service coverage for accepted and rejected Flex inputs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef672d03ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +304 to +305
if (string.Equals(Path.GetExtension(request.SourcePath), ".xml", StringComparison.OrdinalIgnoreCase))
return ibFlexService;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route XML statement-run validation through Flex

When a statement run is created with Broker = "broker" and a Flex .xml file, this extension-based routing is never reached: StatementRunWorkflowService.NormalizeAndValidateAsync validates request.Broker/request.SourcePath first, and StatementReconciliationService treats broker as canonical CSV instead of Flex, so the workflow rejects the XML header before RoutingBrokerStatementService.ImportAsync can choose ibFlexService. This leaves the documented generic-broker-with-Flex-file path working only for direct router calls, not for the workstation/CLI statement-run workflow.

Useful? React with 👍 / 👎.

@rodoHasArrived

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: rodoHasArrived <55965792+rodoHasArrived@users.noreply.github.com>

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts and merged origin/main into this PR branch in commit e85f584.

claude added 2 commits July 1, 2026 21:10
A statement run with Broker='broker' and a Flex .xml file was rejected
by CSV header validation before the import router's extension fallback
could select the Flex parser. StatementReconciliationService now
mirrors the router: canonical broker/custodian kinds with an .xml
source validate and ingest as Flex. Adds validation-service and
end-to-end workflow regression tests for the generic-broker XML path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC
Satisfies the format gate introduced on main: indents the
expression-bodied object initializers in SessionTcaReporterTests and
DataReplacementCostEstimatorTests the way dotnet format expects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5af62ccbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 234 to 237
if (UsesFlexProcessing(normalizedSourceKind, sourcePath) || !UsesCanonicalSchema(normalizedSourceKind, mappingProfileId))
{
var content = File.ReadAllText(sourcePath);
var importId = DeterministicFingerprint.Compute($"{normalizedSourceKind}|{sourcePath}|{content}");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Don't short-circuit Flex case intake

When a Flex file is processed through the generic CLI/orchestrator path, StatementCommands calls IReconciliationCaseIntakeService, which delegates to CreateExternalStatementCasesAsync; this branch now returns RowCount=0, MatchCount=0, and no cases for every Flex XML. As a result --statement-reconcile/--statement-orchestrate can report unresolved=0 even though the preceding Flex import counted rows, so Flex statement breaks are silently skipped outside the bespoke StatementRunWorkflowService path.

Useful? React with 👍 / 👎.

0m,
0m,
ParseDecimal(cash, "amount"),
"cash",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve Flex fee activity types

When a Flex CashTransaction has type="Other Fees", the newly registered IB Flex profile maps that source code to fee, but this parser hard-codes every cash transaction to cash. The statement-run workflow feeds these canonical rows directly into StatementMatchingService, so broker fees are categorized with cash tolerance/break explanations instead of fee handling, corrupting the reconciliation output for fee/tax cash-transaction rows.

Useful? React with 👍 / 👎.

Addresses two review findings on the IB Flex connector:

- Case intake (CLI/orchestrator path) no longer short-circuits Flex
  files to zero rows/cases: Flex sections are read into normalized
  statement rows and matched with the same engine as canonical CSV, so
  --statement-reconcile reports real match/unresolved counts.
- Fee-like CashTransaction types ('Other Fees' etc.) map to the
  canonical 'fee' activity instead of being hard-coded to 'cash', so
  fee rows get fee tolerance/break handling in matching. Dividends,
  deposits, interest, and withholding tax stay 'cash'.

Adds regression tests for both paths (356 reconciliation-area tests
pass).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19c61a0aee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


var importId = duplicateKey;
var normalizedRequest = request.WithSourceFileHash(sourceFileHash);
var rows = ParseRows(document, importId).ToList();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject Flex imports with no supported rows

When an operator imports a Flex XML whose query definition omits Trade, OpenPosition, and CashTransaction sections, this produces an empty row list and ImportAsync still persists a CanonicalStatementImport with NormalizedRowCount=0. The statement-run workflow does not call IbFlexBrokerStatementService.ValidateAsync; it only performs root validation before this import, so a misconfigured Flex query can be recorded as a clean run with no breaks or cases. Please fail the import when rows.Count == 0, matching the condition already reported by ValidateAsync, before saving the import.

Useful? React with 👍 / 👎.

Comment thread src/Meridian/UiServer.cs Outdated
Comment on lines +124 to +125
builder.Services.AddOptions<Meridian.Storage.Services.DataReplacementCostOptions>()
.BindConfiguration(Meridian.Storage.Services.DataReplacementCostOptions.SectionName);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind replacement-cost options in shared UI registration

Fresh evidence in this revision is that the new binding is placed only in UiServer, but the same WorkstationEndpoints are also hosted through the public AddUiSharedServices path, which still only calls AddMarketDataServices and AddWorkstationSharedServices. In that host, TryBuildDataReplacementCostEstimate has no IOptionsMonitor<DataReplacementCostOptions> and falls back to hard-coded defaults, so DataReplacementCost:Enabled=false or custom rate cards in config are ignored outside UiServer. Move/register the binding in the shared UI service registration so every host of the endpoint honors operator config.

Useful? React with 👍 / 👎.

Implements the DuckDB analytics workbench from the brainstorm (idea #5):

- DuckDbQueryService (Meridian.Storage): read-only SQL over the local
  Parquet/JSONL store via an in-memory DuckDB session per query, with
  a meridian_files catalog view (symbol, event type, date, format,
  absolute path, size, event count) for discovery. Session memory and
  thread limits keep analytics off the ingestion hot path.
- SqlStatementGuard: single SELECT-family statement enforcement plus a
  write/config keyword blocklist (COPY/EXPORT/ATTACH/INSTALL/CREATE/
  SET/...), row cap with truncation flag, and query timeout.
- POST /api/workstation/data/query endpoint (rate-limited; guard and
  SQL errors returned in-band as success=false payloads) with options
  bound from the DataQuery configuration section.
- Data screen 'SQL query' panel: statement editor, run button, row
  count/elapsed status, inline error banner, and results table, with a
  dedicated view-model hook and vitest coverage.
- DuckDB.NET.Data.Full added via central package management.

Also addresses two review findings: Flex imports with no supported
rows now fail loudly instead of persisting a clean zero-row run, and
the DataReplacementCost/DataQuery options bindings moved into the
shared workstation service registration so every endpoint host honors
operator configuration.

36 new .NET tests (guard + service including catalog view, truncation,
escaping) and 4 vitest tests; affected suites pass (641 .NET, dashboard
suite green) and the dashboard builds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 991f122c9c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

await CreateCatalogViewAsync(connection, timeoutSource.Token).ConfigureAwait(false);

using var command = connection.CreateCommand();
command.CommandText = sql!;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restrict DuckDB scans to the storage root

When a tenant-scoped workstation user submits a read function such as SELECT * FROM read_csv_auto('/etc/passwd') (or any other absolute path the Meridian process can read), the SQL guard allows it and this line executes it in an unrestricted DuckDB connection. The UI even encourages feeding paths into DuckDB readers, but neither the endpoint nor the service constrains those paths to meridian_files/StorageOptions.RootPath, so the new query workbench can exfiltrate local config, keys, or other host files rather than only the catalogued market-data store.

Useful? React with 👍 / 👎.

Addresses a review finding that reader functions (read_csv_auto etc.)
could open any file the Meridian process can read. Each query session
now sets allowed_directories to the storage root, disables all other
external access (paths, remote endpoints, extension loading), and
locks the configuration so the sandbox cannot be altered for the rest
of the session. Adds regression tests proving reads outside the root
fail and reads inside the root still work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4dcbb9a2bc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

$"SET memory_limit='{options.MemoryLimitMb}MB'; "
+ $"SET threads={options.MaxThreads}; "
+ $"SET allowed_directories=['{storageRoot}']; "
+ "SET enable_external_access=false; "

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep DuckDB file readers usable inside the storage root

When an operator follows the new Data workspace flow and runs read_parquet/read_csv_auto against a path from meridian_files, this setting disables DuckDB external file readers entirely; DuckDB documents enable_external_access=false as blocking readers such as read_csv, read_parquet, and read_json, so allowed_directories never gets to whitelist the storage root. The panel can list catalog metadata, but the advertised ability to scan the local store fails for every in-root data file.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Re: the Codex P2 "Keep DuckDB file readers usable inside the storage root" (DuckDbQueryService.cs):

The described behavior applies to DuckDB before 1.2. Since DuckDB 1.2, allowed_directories exists specifically to whitelist paths while enable_external_access=false — reader functions work inside the whitelisted directories and fail everywhere else. This PR ships DuckDB.NET.Data.Full 1.3.0 (DuckDB 1.3.x), and the behavior is covered by two regression tests that run against the real engine:

  • ExecuteAsync_AllowsReadingFilesInsideStorageRootread_csv_auto on a file under the storage root succeeds with the sandbox active.
  • ExecuteAsync_BlocksReadingFilesOutsideStorageRoot — the same call outside the root fails.

Both pass, so no change is needed; in-root scans remain usable while the P1 exfiltration path stays closed.


Generated by Claude Code

Comment thread src/Meridian.Storage/Query/DuckDbQueryService.cs Dismissed
claude added 2 commits July 1, 2026 22:13
CodeQL flags cs/sql-injection where the data query workbench executes
operator-supplied SQL. That execution is the feature, so the comment
at the flagged line records the containment layers (statement guard,
in-memory session, resource caps, storage-root filesystem sandbox with
locked configuration, endpoint rate limiting) for future audits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC
verify-desktop failed on WorkspaceShellContextServiceTests
.CreateAsync_WhenFixtureModeAndUnreadAlerts_SurfaceDemoEnvironmentAndWarningAlerts,
which asserts against the global NotificationService.Instance and
passed on the three prior commits with identical WPF code (the failing
commit changed only a code comment in Meridian.Storage). Pre-existing
main-side flakiness; rerun-failed-jobs API is unavailable to this
integration (403), so re-triggering via an empty commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011vonQATmobZwPb2KcouHnC
@rodoHasArrived
rodoHasArrived merged commit 9390e1d into main Jul 1, 2026
25 checks passed
@rodoHasArrived
rodoHasArrived deleted the claude/prioritize-high-value-code-88hyfu branch July 17, 2026 03:39
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.

5 participants