Refactor UI service layer and dashboard: dedupe, typed status, consistent error/API contracts - #2203
Merged
Conversation
…501 convention, notification tracing - ApiClientService: extract shared SendAsync/SendWithResponseAsync/CreateJsonContent helpers, removing near-identical try/catch/wrap logic across five methods (#1). - CoveredCallEndpoints: return 501 (not 503) when the backtest service is not registered, matching the shared not-registered convention; keep 503 only for the genuine ConfigurationException path (#2). - BackfillService: replace raw status string literals with the existing BackfillJobStatus / SymbolBackfillStatus contract constants (#6). - NotificationService: trace discarded notifications when no real surface is wired, instead of silently dropping them (#8). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqmPtfK56LqvGdk5vhZodA
- security-details-tracker: serialize optimistic override patches with a generation guard so a stale or failed earlier patch cannot clobber a newer edit, and roll the optimistic value back to the last-known-good state when the server rejects it (#11). - trial-balance-screen / accounting-screen: replace two divergently-named local row-count thresholds (both 40) with the shared DENSE_VIRTUALIZATION_THRESHOLD (#12). - accounting-screen: render error states through the shared StatusBanner instead of bare danger divs, matching the error-state UX used elsewhere (#16). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqmPtfK56LqvGdk5vhZodA
- HttpClientConfiguration: delegate retry/circuit-breaker to the single SharedResiliencePolicies definition in Meridian.Infrastructure instead of maintaining a byte-identical private copy. Both this project and the WPF app already reference Infrastructure, so the 'keep in sync' comment was stale; timeouts stay local because the desktop backfill client keeps its 60-minute LongTimeout (#3). - PortfolioImportService: move the curated offline index-constituent fallback lists into a documented, data-driven IndexConstituentCatalog (canonical ids + alias map), so the partial samples are easy to extend without editing import logic; behaviour unchanged (#5). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqmPtfK56LqvGdk5vhZodA
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
Satisfies the dotnet format whitespace gate (one collection element per line), which was failing verify-fast/verify-dotnet/quality-gate before the build and tests could run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqmPtfK56LqvGdk5vhZodA
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
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
Targeted maintainability fixes across
Meridian.Ui.Services,Meridian.Ui.Shared, and the browser dashboard. All changes preserve observable behavior.Backend (
Meridian.Ui.Services/Meridian.Ui.Shared)SendAsync<T>,SendWithResponseAsync<T>, andCreateJsonContenthelpers; the near-identical try/catch/wrap logic that was repeated across five methods (GetAsync,GetWithResponseAsync,PostAsync,PostWithResponseAsync,DeleteWithResponseAsync) now lives in one place. Serialization still happens inside the try block, so error handling is unchanged.CoveredCallEndpointsnow returns 501 (matching the convention used by every other UI endpoint) instead of 503 whenICoveredCallBacktestServiceis not registered. The genuineConfigurationExceptionpath keeps its 503, and.Produces(...)declarations were updated accordingly.HttpClientConfigurationnow delegates retry/circuit-breaker to the singleSharedResiliencePoliciesdefinition inMeridian.Infrastructure. Both this project and the WPF app already reference Infrastructure directly, so the "keep in sync" comment was stale; the byte-identical private copies were removed. Local timeouts are retained because the desktop backfill client keeps its 60-minuteLongTimeout(Infrastructure'sLongTimeoutis 60 seconds).PortfolioImportServicemoved into a documented, data-drivenIndexConstituentCatalog(canonical ids + alias map). The lists are clearly labeled partial samples and easy to extend without touching import logic; behavior is identical.BackfillServicenow uses the existingBackfillJobStatus/SymbolBackfillStatuscontract constants instead of scattered"Running"/"Paused"/"Completed"literals.Debug.WriteLine(matchingNotificationServiceBase), so a host that forgets to wire a real notification surface gets a diagnostic signal instead of silent loss.Dashboard (
src/Meridian.Ui/dashboard)security-details-trackernow serializes override patches with a generation guard so a slow/failed earlier patch cannot clobber a newer edit or mask its own failure, and rolls the optimistic value back to the last-known-good state when the server rejects it.40, inaccounting-screenandtrial-balance-screen) now reference the sharedDENSE_VIRTUALIZATION_THRESHOLD.accounting-screenerror states now render through the sharedStatusBanner(matchingdata-screenand the rest of the app) instead of barerole="alert"danger divs.Deferred (documented, not in this PR)
These items are large, cross-cutting, or contract-level refactors that cannot be done as a safe, behavior-preserving change in one pass and warrant their own scoped work:
StorageOptimizationAdvisorService.cs(~1,553 lines) by concern.Result<T>convention across the service layer.AccountingConfigurationService.GetWorkspaceAsyncmulti-nullable/param-ordering — anIAccountingConfigurationServicecontract change touching many endpoints.WorkstationEndpoints.cs— flagged as a watch-item only (already split via partial class); no action taken.accounting-screen.tsx/settings-screen.tsx/accounting-screen.view-model.tsfiles.@/typesfrom the shared contract layer instead of hand-mirroring backend DTOs.Reason
Addresses a maintainability/consistency review of the UI service and dashboard layers: duplicated logic, contract inconsistencies (status codes, raw status strings), a silent failure mode, an optimistic-update race, and duplicated/divergent constants and error-state UX.
Testing performed
bash scripts/ci.shcompleted successfully — not run: no .NET SDK is available in this environment; C# changes rely on the GitHubquality-gate.typecheck:strictclean;security-details-tracker,accounting-screen(47),trial-balance-screen(component + view-model),w4-acceptance-parity, anddesign-system-contracttests all pass; ESLint on touched files shows only pre-existing warnings.quality-gatepassed — pending CI.Safety review
mainmainwas performedGovernance changes
🤖 Generated with Claude Code
https://claude.ai/code/session_01GqmPtfK56LqvGdk5vhZodA
Generated by Claude Code