Skip to content

[refdata,synthetic,qt] Add curve_role and yield_curve_process_type entities - #1610

Merged
mcraveiro merged 4 commits into
mainfrom
feature/tick-batch-publishing
Jul 16, 2026
Merged

[refdata,synthetic,qt] Add curve_role and yield_curve_process_type entities#1610
mcraveiro merged 4 commits into
mainfrom
feature/tick-batch-publishing

Conversation

@mcraveiro

Copy link
Copy Markdown
Contributor

Summary

Data-model foundation for the IR curve family feed (tick-batch-publishing task): two new full-stack codegen entities replacing hardcoded-classification/inline-CHECK designs with proper, FK-validated, describable reference data, plus a new FK column needed for swap fixed-leg schedule building. The ir_curve_feed producer itself is not part of this PR -- follow-on work.

Changes

  • New ores.refdata.curve_role entity: classifies instrument_code by pricing role (DEPOSIT/FRA/SWAP/NONE sentinel), replacing a hardcoded-switch design that was rejected during design review.
  • instrument_code gets a new curve_role FK column, insert-trigger validated; all 115 catalogue rows re-seeded (DEPO/FRA/IRS tagged, rest default NONE).
  • New ores.synthetic.yield_curve_process_type entity: replaces ir_curve_generation_config.process_type's inline CHECK constraint with a proper FK-validated catalogue (VASICEK/CIR/HULL_WHITE).
  • ir_curve_generation_config gets a new fixed_leg_payment_frequency_code FK to the existing ores.refdata.payment_frequency, needed to build a Swap curve-template entry's fixed-leg schedule.
  • Both new entities are full-stack: domain/repository/service/protocol, NATS handler + eventing registrars (hand-wired into each component's aggregate registrar), and Qt list/detail/history windows wired into their plugin's menu (Classifications submenu for curve_role, Advanced menu for yield_curve_process_type).
  • Verification: local build clean (linux-clang-debug-make) across all 8 touched targets (ores.refdata.api/core/service, ores.synthetic.api/core/service, ores.qt.refdata, ores.qt.synthetic) -- 0 errors, 0 warnings. db recreate verified schema + seed data end-to-end (curve_role: 4 rows, yield_curve_process_type: 3 rows, instrument_code.curve_role: DEPO/FRA/IRS correctly tagged).

Traceability

Artefact Link ID
Story IR Rates synthetic data generation 3ECC4BA8-6ACA-4028-9E42-AE7F25FC3B98
Task Tick-batch publishing and persistence for curve instruments 615FD100-BCD9-4CF5-B9DF-197447769AA9
Environment eager_maxwell

🤖 Generated with Claude Code

mcraveiro and others added 4 commits July 16, 2026 16:28
…trap capture

tick-batch-publishing was closed via PR #1597, which shipped only the
payment_frequency prerequisite, not the ir_curve_feed producer its own
Acceptance requires. Reopened, scoped to Phase 1 (synchronous family
feed) only, with an Analysis section recording the phase-split design
(family feed vs. per-point disaggregation, deferred to the existing
stochastic-tick-arrival-times story), the two-subject NATS design, and
why no batch/generation identifier is needed (checked against FIX and
Refinitiv conventions -- neither has one; curves are reconstructed
downstream via as-of queries).

Added curve-snapshot-builder-viewer task: a minimal as-of query + plain
tenor table over raw ticks, so the family feed's output can actually be
seen once it exists.

Filed an inbox capture for the bootstrapping + official-curve-republish
gap this story deliberately doesn't cover.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nstruments

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tities

Full-stack codegen entities (domain/repository/service/protocol/NATS
messaging/eventing/Qt) for two data-model gaps surfaced while designing
the IR curve family feed:

- ores.refdata.curve_role: classifies instrument_code by its role when
  deriving a rate from a curve (DEPOSIT/FRA/SWAP/NONE sentinel), FK-
  validated new column on instrument_code, replacing a hardcoded-switch
  design. Seeded via Foundation layer; DEPO/FRA/IRS instrument codes
  tagged accordingly, all others default NONE.
- ores.synthetic.yield_curve_process_type: replaces
  ir_curve_generation_config.process_type's inline CHECK constraint
  with a proper, FK-validated, describable catalogue entity
  (VASICEK/CIR/HULL_WHITE).
- ir_curve_generation_config.fixed_leg_payment_frequency_code: new FK
  to the existing ores.refdata.payment_frequency, needed to build a
  Swap curve-template entry's fixed-leg schedule.

Both new entities wired into their component's NATS handler/event
registrars and Qt plugin menus (Classifications submenu for curve_role,
Advanced menu for yield_curve_process_type), following the tenor_unit
reference pattern.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Story-ID: 3ECC4BA8-6ACA-4028-9E42-AE7F25FC3B98
Task-ID: 615FD100-BCD9-4CF5-B9DF-197447769AA9
@mcraveiro

Copy link
Copy Markdown
Contributor Author

@claude please review this pull request

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude is reviewing this PR

Tasks

  • Gather context (PR description, changed files list)
  • Read the diff / changed files list
  • Review new curve_role entity (SQL, domain, repository, service, messaging, Qt UI)
  • Review new yield_curve_process_type entity (SQL, domain, repository, service, messaging, Qt UI)
  • Review instrument_code.curve_role FK addition and re-seeded data
  • Review ir_curve_generation_config.fixed_leg_payment_frequency_code FK addition
  • Post final review feedback

Running three parallel deep-dive passes (SQL layer, C++ domain/repository/service/messaging layer, Qt UI layer), comparing against the existing payment_frequency/InstrumentCode patterns for consistency.

View job run

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished reviewing this PR

Todo list

  • Gather context (PR description, changed files, CLAUDE.md)
  • Review SQL migrations (create/drop/populate, triggers, FKs, seed ordering)
  • Review new curve_role entity (refdata domain/repository/service/messaging)
  • Review new yield_curve_process_type entity (synthetic domain) + ir_curve_generation_config FK additions
  • Review refdata Qt UI wiring (curve_role, instrument_code)
  • Review synthetic Qt UI wiring (yield_curve_process_type, ir_curve_generation_config)
  • Post final review feedback

This is a large, mostly-codegen PR and the bulk of the generated full-stack scaffolding (domain/repository/service/messaging for both new entities, curve_role's Qt wiring, and instrument_code's new curve_role combo) is solid and follows the established sibling-entity pattern correctly. Three concrete issues turned up, ranked by severity:

1. High — process_type / fixed_leg_payment_frequency_code are still free-text fields, not FK combos

projects/ores.qt/synthetic/ui/IrCurveGenerationConfigDetailDialog.ui:77 (processTypeEdit) and :164 (fixedLegPaymentFrequencyEdit) are both QLineEdit, not QComboBox. IrCurveGenerationConfigDetailDialog.cpp treats both purely as text (setText/text().trimmed().toStdString() at lines 110‑245) — there's no populateDynamicCombo<T>() call, no fetch from fetch_yield_curve_process_types/a payment-frequency fetcher, no currentData() read-back, unlike the sibling FK-combo pattern used elsewhere (e.g. CrmDriverPairDetailDialog.cpp:188, or curve_role's own combo wiring into InstrumentCodeDetailDialog.cpp:192-212 added in this very PR).

This directly undercuts the PR's own stated purpose — the yield_curve_process_type domain header (ores.synthetic.api/domain/yield_curve_process_type.hpp:29-33) says it "replaces an inline CHECK constraint ... with a proper, FK-validated, describable catalogue row" — but that migration only happened server-side. In the UI, a user can type "vasiceck" or "Anual" into these fields, pass client-side validation (validateInput() only checks non-empty), and get a raw server error back instead of picking from a dropdown. fixedLegPaymentFrequencyCode is a brand-new field added by this PR, so it was never a combo to "keep" — it should have been built as one from the start, the same way curve_role's combo was on instrument_code.

Fix this →

2. Medium — new drop scripts aren't wired into teardown orchestration

refdata_curve_roles_drop.sql/refdata_curve_roles_notify_trigger_drop.sql and synthetic_yield_curve_process_types_drop.sql/..._notify_trigger_drop.sql were added but are never \ir'd from projects/ores.sql/drop/refdata/refdata_drop.sql or projects/ores.sql/drop/synthetic/synthetic_drop.sql (contrast with the instrument_codes/asset_class_codes pair at refdata_drop.sql:114-118, which is correctly wired). db recreate (the workflow the PR description says was used for verification) won't actually drop these two tables/functions/triggers, so schema changes on these tables won't be picked up on a repeat recreate cycle — plausible that this slipped through because create table if not exists is idempotent and silently no-ops instead of erroring.

Fix this →

3. Medium — new write/delete permissions never seeded into the RBAC catalog

The handlers reference refdata::curve_roles:write/:delete (curve_role_handler.hpp:99,157) and synthetic::yield_curve_process_types:write/:delete (yield_curve_process_type_handler.hpp:101,168), but none of these four permission codes are registered via ores_iam_permissions_upsert_fn in projects/ores.sql/populate/iam/iam_permissions_populate.sql — every other codegen'd entity (e.g. instrument_codes at lines 207-209, ir_curve_generation_configs at lines 407-409) gets its own read/write/delete triplet there. This is currently masked by SuperAdmin/TenantAdmin's '*' wildcard and RefdataService/SyntheticService's component wildcards, so nothing breaks today, but no custom/scoped role can ever be granted fine-grained access to these two entities since the permission codes don't exist in the catalog (ores_iam_role_permissions_assign_fn looks the code up and raises if it's missing).

Fix this →

Minor / non-blocking

  • In curve_role_registrar.cpp's registration and the corresponding includes in registrar.cpp, event_registrar.cpp, and LookupFetcher.hpp/.cpp, the new curve_role lines are inserted out of the otherwise-alphabetical order used everywhere else in those files (e.g. dropped in next to tenor_unit instead of near currency_*/counterparty). Purely cosmetic.

What checked out clean

  • SQL schema/trigger/validation-function generation for both curve_role and yield_curve_process_type (bitemporal versioning, unique indexes, NOTIFY triggers, validate-fn bootstrap pass-through) — correct and consistent with the established template.
  • Seed/population ordering: curve_role before instrument_code in both refdata_create.sql and foundation_populate.sql; yield_curve_process_type before ir_curve_generation_config in synthetic_create.sql; fixed_leg_payment_frequency_code default ("Annual") matches the actual seeded payment_frequency code casing.
  • curve_role's full Qt stack (CurveRoleController/DetailDialog/MdiWindow/ClientCurveRoleModel) and its new combo on InstrumentCodeDetailDialog — verified correctly wired, correct column indices, correct read-back on save, correct menu placement under Classifications.
  • YieldCurveProcessTypeController/DetailDialog/MdiWindow/ClientYieldCurveProcessTypeModel and SyntheticPlugin menu wiring under Advanced — correct.
  • NATS handler/protocol/registrar/eventing wiring for both entities — correct request/response shapes, correct subject names, correct hand-wiring into registrar.cpp/event_registrar.cpp.
  • Test coverage: neither new entity has a repository test file, but this matches the existing (inconsistent) convention — several other recently-added simple lookup entities (instrument_code, payment_frequency, tenor_kind, asset_class_code) also lack one, so this isn't a regression introduced by this PR.

View job run

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished the review

  • Gather context (diff, CLAUDE.md, repo conventions)
  • Understand the request (code review)
  • Review SQL migrations (curve_role, yield_curve_process_type)
  • Review refdata/synthetic domain, repository, service, messaging layers
  • Review Qt UI wiring (controllers, models, dialogs)
  • Post review feedback

Summary

Solid, well-structured PR that follows the established codegen pattern closely — the SQL (refdata_curve_roles_create.sql, synthetic_yield_curve_process_types_create.sql), triggers, FK-validation functions, and seed data are consistent with the equivalent boilerplate for other reference-data catalogues in this codebase (e.g. payment_frequency, tenor_unit). instrument_code's new curve_role column is correctly tagged for the 3 non-NONE rows (IRS→SWAP, FRA→FRA, DEPO→DEPOSIT) and the FK/populate ordering is correct in refdata_create.sql / foundation_populate.sql.

One correctness concern found that's worth addressing before/along with the follow-on producer work, plus a couple of minor cleanup items.

Findings

1. process_type case mismatch with process_factory (real, but latent since the producer isn't wired yet)

ir_curve_generation_config.hpp:87 now defaults process_type to "VASICEK" (uppercase), and the new catalogue seeds uppercase codes (VASICEK/CIR/HULL_WHITE in synthetic_yield_curve_process_types_populate.sql, and the updated CHECK constraint process_type <> 'CIR' in synthetic_ir_curve_generation_configs_create.sql).

However, ores.analytics.quant's process_factory::make_yield_curve_process() (projects/ores.analytics.quant/src/service/process_factory.cpp:75-80) still dispatches on lowercase literals: "vasicek", "cir", "hull_white". Any value round-tripped from the new catalogue straight into that factory will fall through to the throw std::invalid_argument("...unrecognised process_type...") at line 84-86.

This doesn't break anything in this PR — nothing yet calls make_yield_curve_process() with a persisted ir_curve_generation_config.process_type (confirmed via grep; the only callers are process_factory_tests.cpp, still using lowercase literals). But it's a landmine for the "follow-on" ir_curve_feed producer work referenced in the PR description: whoever wires that up will hit this mismatch unless it's normalized (either lowercase the value before the factory call, or update process_factory to match on the new uppercase catalogue codes). Worth a note on the task/backlog item so it isn't rediscovered the hard way.

Fix this →

2. Orphaned IrCurveGenerationConfigHistoryDialog files left behind

projects/ores.qt/synthetic/src/IrCurveGenerationConfigController.cpp was migrated from the bespoke IrCurveGenerationConfigHistoryDialog to the generic HistoryDialog (matching the pattern from PR #1609), but IrCurveGenerationConfigHistoryDialog.hpp, .cpp, and .ui under projects/ores.qt/synthetic/{include,src,ui} weren't deleted. Nothing else in the tree references them anymore (verified via grep), so they're now dead code still picked up by the CMakeLists.txt GLOB_RECURSE. Recommend deleting the three orphaned files.

Fix this →

3. Minor: populate-script placement nit

synthetic_yield_curve_process_types_populate.sql is \ir'd from foundation_populate.sql under the -- Reference Data Lookup Tables -- section header (between payment_frequencies and tenors), even though it's an ores.synthetic-owned entity, not refdata. It's the only ores.synthetic populate script in that whole section. Functionally harmless (no other synthetic entity depends on load order there), just a bit of a layering surprise for a reader scanning that section expecting only refdata scripts.


No other issues found — the rest of the new files (domain/repository/service/messaging layers for both entities, Qt models/controllers/dialogs) follow the established generated-entity pattern consistently.

View job run · Branch: feature/tick-batch-publishing

@mcraveiro
mcraveiro merged commit e4176bf into main Jul 16, 2026
5 checks passed
@mcraveiro
mcraveiro deleted the feature/tick-batch-publishing branch July 16, 2026 23:22
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