Fix spec dashboard overview grouping for C# management emitter - #11709
Fix spec dashboard overview grouping for C# management emitter#11709Wei Hu (live1206) wants to merge 2 commits into
Conversation
|
No changes needing a change description found. |
There was a problem hiding this comment.
Pull request overview
This PR updates the spec dashboard’s coverage overview to aggregate coverage by a logical emitter “display name” (e.g., grouping data-plane and management-plane C# emitters together) and adds a regression test intended to prevent the C# management emitter from appearing as a separate language card.
Changes:
- Group overview cards by a logical key derived from the emitter display name rather than the raw emitter package name.
- Track/display a grouped display name per overview entry.
- Add a regression test for the C# management/data-plane grouping scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/spec-dashboard/src/components/coverage-overview.tsx | Changes overview aggregation from per-emitter-package to per-display-name grouping. |
| packages/spec-dashboard/src/apis.test.ts | Adds a regression test rendering CoverageOverview to validate grouping behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/spec-dashboard/src/components/coverage-overview.tsx:72
- The PR description (and linked issue) also calls out the management-plane table header showing a raw package name. This change only affects the overview grouping;
DashboardTableheader rendering is unchanged, so the table header may still fall back to the emitter package name whengeneratorMetadata.name/emitterDisplayNamesaren’t available. Either update the PR description to scope it to the overview-only fix, or include a table-header fix (e.g., derive a friendly name for*-mgmtpackages or ensure the mgmt emitter is always mapped viaemitterDisplayNames).
// Aggregate scenarios per logical emitter language across all summaries.
// This keeps emitters that share the same display name (for example C# data-plane
// and management-plane emitters) grouped into a single overview card.
packages/spec-dashboard/src/components/coverage-overview.tsx:134
- The overview aggregation does two linear scans over
summary.generatorReportspergroupKey(to computefirstReportand then again to computedisplayName). This makes the grouping work O(n²) per summary and adds avoidable complexity. SincegroupKeyis already the resolved display name, you can initialize the entry without re-scanning the reports.
const firstReport = Object.entries(summary.generatorReports).find(
([emitterName, report]) =>
getEmitterOverviewKey(emitterName, report, emitterDisplayNames) === groupKey,
)?.[1];
|
You can try these changes here
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/spec-dashboard/src/components/coverage-overview.tsx:133
- Inside the
if (!emitterMap.has(groupKey))block,Object.entries(summary.generatorReports).find(...)is executed twice with the same predicate. This adds unnecessary work and makes the code harder to follow; hoist the result into a single variable and reuse it for both thefirstReportand the emitter name used to computedisplayName.
const firstReport = Object.entries(summary.generatorReports).find(
([emitterName, report]) =>
getEmitterOverviewKey(emitterName, report, emitterDisplayNames) === groupKey,
)?.[1];
Summary
Fixes the overview bug where the management C# emitter was shown as a separate language in the coverage overview and as a raw package name in the table header.
This change groups emitters by their logical display name so data-plane and management-plane C# coverage are aggregated into a single C# overview card.
Changes
Testing
RUN v4.1.10 /home/live1206/typespec/packages/spec-dashboard
Test Files 1 passed (1)
Tests 14 passed (14)
Start at 05:57:07
Duration 2.76s (transform 196ms, setup 0ms, import 2.54s, tests 34ms, environment 0ms)
Fixes #11699