Skip to content

Recs rebuild WS1a: unified Recommendations reader + de-dupe (data layer)#1060

Merged
erikdarlingdata merged 1 commit into
devfrom
feature/recs-ws1a-unified-reader
Jun 5, 2026
Merged

Recs rebuild WS1a: unified Recommendations reader + de-dupe (data layer)#1060
erikdarlingdata merged 1 commit into
devfrom
feature/recs-ws1a-unified-reader

Conversation

@erikdarlingdata
Copy link
Copy Markdown
Owner

Scope

WS1a of the recommendations-engine rebuild: the data layer only for the unified Recommendations surface. No XAML / no WPF control / no tab wiring (that is WS1b, a later PR), so nothing renders yet — this is foundational, like prior PRs. Dashboard-only.

Builds a unified reader + view-model + de-dupe over the two producers for a server:

  • Engine findings (config.analysis_findings) read via the existing SqlServerFindingStore.GetRecentFindingsAsync (no new raw SQL for the findings read).
  • Legacy config recs (config.critical_issues) read via the existing DatabaseService.GetCriticalIssuesAsync.

Implements decisions D1 (single surface / hybrid producers), C3 (concrete cross-store de-dupe), D3 (advise/copy-paste + Apply tiering — Apply carried via the persisted action, surfaced in WS1b).

Key design note (verified against the code)

GetRecentFindingsAsync returns remediation_action_json (the built RemediationAction, D2) but NOT the ephemeral DrillDown. So on read the engine Setting and copy-paste SQL are derived from the persisted RemediationAction (FactKey=="RCSI" → Rcsi; DbConfigTargets → AutoShrink/AutoClose/PageVerify), not from a drill-down that is absent. The copy-paste ALTER DATABASE statements are rebuilt from those persisted targets with the same QUOTENAME-equivalent bracketing + SET-clause literals the executor uses.

File:line changelog

New files (4; +1067):

  • Dashboard/Services/Recommendations/RecommendationItem.cs:1 — the unified VM (plain DTO, no WPF dep) + the CanonicalSeverity (Info/Warning/Critical), RecommendationSource (Engine/Legacy), RecommendationSetting (None/AutoShrink/AutoClose/QueryStore/Rcsi/PageVerify) enums. Fields: CanonicalSeverity, RawSeverity, Database, Title, ProblemArea, AdviceText, CopyPasteSql, Remediation (RemediationAction?), Source, StoryPathHash, Setting.
  • Dashboard/Services/Recommendations/RecommendationDeduper.cs:1 — the PURE static core: FromEngineSeverity (>=1.5 Critical / >=0.75 Warning / else Info, :30), FromLegacySeverity (text → band, :46), Merge (the de-dupe over two mapped lists, :79), PreferEngineFor (collision precedence, :135), NormalizeDatabase (trim/lower, :170), CompareForDisplay (severity desc total order, :181).
  • Dashboard/Services/Recommendations/RecommendationsReader.cs:1GetRecommendationsAsync(serverId, serverName, hoursBack, limit) (:59) reads both stores, maps, de-dupes, sorts. MapEngineFinding (:83), MapLegacyIssue (:111), SettingFromAction (:145), SettingFromDbConfig (:177), SettingFromLegacy (ALTER-keyword parse for the two config problem-areas only, :194), BuildCopyPasteFromAction (:225).
  • Dashboard.Tests/RecommendationDeduperTests.cs:1 — 41 xUnit tests (no DB).

No existing files modified (git diff --stat vs HEAD is empty).

De-dupe (C3) summary

  • Key = (normalized database [trim/lower], Setting). Setting=None rows never de-dupe (memory pressure, server-level MAXDOP/CTFP, dumps, CPU/wait findings) — all pass through.
  • Engine Setting from the persisted action; legacy Setting only for problem_area ∈ {Database Configuration, Query Store Configuration}, derived from the investigate_query ALTER keyword (AUTO_SHRINK/AUTO_CLOSE/QUERY_STORE) — the free-text message is never parsed.
  • Collision precedence: AutoShrink/AutoClose → Engine (Apply-capable); QueryStore → Legacy (engine has no QS Apply today). Rcsi/PageVerify are engine-only and never collide.

Test counts (REAL — dotnet test --no-build -c Debug)

  • Build: dotnet build PerformanceMonitor.sln -c Debug0 errors (1 pre-existing warning in RemediationTests.cs, not from this PR).
  • Dashboard.Tests: Passed 279, Failed 0, Skipped 0 (of which 41 are the new RecommendationDeduperTests).
  • Lite.Tests: Passed 360, Failed 0, Skipped 0.

New-test coverage: AUTO_SHRINK in both stores same DB → one row Source=Engine; AUTO_CLOSE same; QUERY_STORE in both (synthesized engine QS row) → one row Source=Legacy; same setting different DBs → both kept; memory-pressure legacy (None) + engine CPU finding → both kept; severity boundary cases (0.74→Info, 0.75→Warning, 1.49→Warning, 1.5→Critical, plus the text mapping); plus the setting/copy-paste mapper helpers.

Needs integration verification (live DB — NOT unit-tested here)

  • The actual two-store read end-to-end: RecommendationsReader.GetRecommendationsAsync against a real config.analysis_findings + config.critical_issues (the unit tests exercise the pure de-dupe + the mappers with synthesized inputs, never the DB).
  • That a real engine DB_CONFIG / RCSI finding round-trips its remediation_action_json such that SettingFromAction + BuildCopyPasteFromAction produce the expected Setting and ALTER text from the persisted action on read.
  • That a real legacy AUTO_SHRINK/AUTO_CLOSE/QUERY_STORE row carries the expected problem_area + investigate_query keyword for the de-dupe to fire (and a real per-DB AUTO_SHRINK present in BOTH stores collapses to the single Engine row).
  • Sort/severity-band behavior over real mixed-producer data.

🤖 Generated with Claude Code

Foundational data layer for the unified Recommendations surface (no XAML / no
WPF control / no tab wiring -- that is WS1b). Dashboard-only.

- RecommendationItem VM (+ CanonicalSeverity/RecommendationSource/
  RecommendationSetting enums): plain DTO, no WPF dependency, unit-testable.
- RecommendationDeduper: PURE static severity mapping + (db, Setting) cross-store
  de-dupe (C3). AutoShrink/AutoClose collision keeps Engine; QueryStore keeps
  Legacy; Setting=None never de-dupes.
- RecommendationsReader: reads both stores via the existing
  SqlServerFindingStore.GetRecentFindingsAsync + DatabaseService.
  GetCriticalIssuesAsync (no new raw SQL), maps each row, de-dupes, sorts.
  Engine Setting + copy-paste derive from the PERSISTED RemediationAction
  (drill-down is not returned on read); legacy Setting from the investigate_query
  ALTER keyword for the two config problem-areas only.
- 41 xUnit tests (no DB) covering de-dupe collisions, pass-through, ordering,
  severity boundary cases, and the setting/copy-paste mappers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@erikdarlingdata erikdarlingdata merged commit 730c281 into dev Jun 5, 2026
2 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