Conversation
Introduces an ExportManifest RPC on the ManifestHistoryService that queries downstream services for their current resources and assembles a complete Manifest proto. This enables migrating tenants configured via direct gRPC calls into the manifest-first workflow. Key design decisions: - Collector interfaces per service section (instruments, account types, sagas, market data, organizations, internal accounts, operational gateway) allow the export to be assembled incrementally - Sections without live service collectors (valuation_rules, mappings, party_types, payment_rails, seed_data) fall back to the last applied manifest version - Collector errors produce warnings and graceful fallback rather than failing the entire export - Response includes section_sources map documenting data provenance and SHA-256 checksum for content verification - Read-only operation: no mutations, no manifest version created
📝 WalkthroughWalkthroughIntroduces a manifest export feature via new RPC method Changes
Sequence DiagramsequenceDiagram
participant Client
participant HistoryHandler
participant ExportService
participant Collector
participant HistoryService
Client->>HistoryHandler: ExportManifest(includeSections, version)
HistoryHandler->>ExportService: Export(includeSections, version)
ExportService->>ExportService: parseSections(includeSections)
ExportService->>HistoryService: GetManifestByVersion(version)
HistoryService-->>ExportService: fallbackManifest
ExportService->>Collector: ListInstruments/ListAccountTypes/etc
alt Collector Success
Collector-->>ExportService: live data
ExportService->>ExportService: merge live data into manifest
else Collector Failure
ExportService->>ExportService: record warning, use fallback section
end
ExportService->>ExportService: manifestChecksum(manifest)
ExportService-->>HistoryHandler: ExportResult
HistoryHandler->>ExportService: ToProtoResponse()
HistoryHandler-->>Client: ExportManifestResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approve. One non-blocking suggestion on checksum determinism: protojson.Marshal is not guaranteed deterministic across builds or protobuf library versions. For this PRs scope (single-process content verification) this is fine. But if checksums are later stored and compared across service restarts/upgrades, consider switching to proto.MarshalOptions{Deterministic: true} with the binary wire format, which is guaranteed stable for identical schema+data. See summary comment for full review.
Claude Code ReviewCommit: SummaryClean implementation of the ExportManifest RPC. The collector interface pattern is well-designed: each downstream service gets a focused interface, nil collectors gracefully fall back to the stored manifest, and collector errors produce warnings rather than failing the whole export. The handler integration is minimal and correct -- the new constructor injects the exporter without disturbing existing RPCs. This satisfies the requirement for task Risk Assessment
Findings
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/control-plane/internal/manifest/export.go (1)
326-361: Consider partial success for multi-call collectors.For
collectMarketData, ifListMarketDataSourcessucceeds butListMarketDataSetsfails, the entire section falls back. This loses the successfully retrieved sources. The same pattern exists incollectOperationalGateway. This is a reasonable trade-off for consistency, but worth documenting if intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/control-plane/internal/manifest/export.go` around lines 326 - 361, collectMarketData currently treats any error from ListMarketDataSources or ListMarketDataSets as a full failure and falls back, which discards partial successes; update collectMarketData to preserve successful sub-results (set Sources if ListMarketDataSources succeeded, set Datasets if ListMarketDataSets succeeded), aggregate and append a warning for the failing call(s) (using the existing warning construction), set result.Manifest.MarketData to a MarketDataConfig with whatever subfields succeeded, and annotate result.SectionSources["market_data"] to reflect partial vs full live data (e.g., "live:market-information:partial" or similar) so callers can distinguish partial success; apply the same partial-success pattern to collectOperationalGateway where multiple List* methods are invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@services/control-plane/internal/manifest/export.go`:
- Around line 326-361: collectMarketData currently treats any error from
ListMarketDataSources or ListMarketDataSets as a full failure and falls back,
which discards partial successes; update collectMarketData to preserve
successful sub-results (set Sources if ListMarketDataSources succeeded, set
Datasets if ListMarketDataSets succeeded), aggregate and append a warning for
the failing call(s) (using the existing warning construction), set
result.Manifest.MarketData to a MarketDataConfig with whatever subfields
succeeded, and annotate result.SectionSources["market_data"] to reflect partial
vs full live data (e.g., "live:market-information:partial" or similar) so
callers can distinguish partial success; apply the same partial-success pattern
to collectOperationalGateway where multiple List* methods are invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 841c8e9a-50bb-4e17-94c1-de0cb583df2e
📒 Files selected for processing (6)
api/proto/meridian/control_plane/v1/manifest_history_service.protoservices/control-plane/internal/manifest/export.goservices/control-plane/internal/manifest/export_test.goservices/control-plane/internal/manifest/grpc_handler.goservices/control-plane/internal/manifest/grpc_handler_test.goservices/control-plane/service/manifest_history.go
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
bjcoombs
deleted the
045-manifest-source-of-truth--14--export-manifest-rpc
branch
1 participant
Summary
ExportManifestRPC toManifestHistoryServicefor reconstructing a manifest from live service stateChanges Made
Proto (
manifest_history_service.proto):ExportManifestRequestwithinclude_sectionsfilter and optionalmanifest_versionfor fallback sourceExportManifestResponsewith reconstructedManifest,exported_attimestamp, SHA-256checksum,section_sourcesprovenance map, andwarningsfor partial failuresExport Service (
export.go):InstrumentCollector,AccountTypeCollector,SagaCollector,MarketDataCollector,OrganizationCollector,InternalAccountCollector,OperationalGatewayCollector) abstract downstream service queriesExportCollectorsstruct groups optional collectors; nil collectors fall back to the stored manifestvaluation_rules,mappings,party_types,payment_rails) always source from fallbackHandler (
grpc_handler.go):NewHistoryHandlerWithExportconstructor injects theExportServiceExportManifestRPC returnsUnimplementedwhen no exporter is configuredService registration (
manifest_history.go):ManifestHistoryServiceConfig.ExportCollectorsfield to wire up live collectors at startupTesting
Task Master
Task
045-manifest-source-of-truth.14- Implement ExportManifest RPC