Migrate field sums in grouped table and roadmap views - #75
Conversation
Field sums were only written for boards, while GitHub truncates multi-value summaries and exposes the same control for grouped tables and roadmaps. Read and synchronize the scoped submenu so multiple fields and empty selections round-trip without changing unrelated display options. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
The updated acceptance criteria require a repeatable operator scenario in addition to automated coverage. Record fixture preparation, exact commands, drift detection, idempotent repair, evidence, and cleanup, and correct the incompatible project-number conflict option guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
The existing browser-e2e workflow must cover newly migrated view settings without introducing a separate scenario. Add fixture and snapshot gates, strict View verification, UI drift and repair checks, and cleanup inventory to the standard flow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Extends browser-assisted field-sum migration to grouped Table and Roadmap views.
Changes:
- Captures and applies complete field-sum selections.
- Expands fixtures and browser round-trip coverage.
- Updates E2E validation and documentation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Ghpmv.Core/Browser/ViewUiExporter.cs |
Reads checked field-sum entries. |
src/Ghpmv.Core/Browser/ViewUiImporter.cs |
Applies sums across supported layouts. |
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs |
Adds grouped field-sum fixture views. |
src/Ghpmv.Core/Fixtures/FixtureProjectBuilder.cs |
Adds a second number field. |
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs |
Updates field-sum documentation. |
tests/Ghpmv.Browser.Tests/ViewUiLogicTests.cs |
Tests layout planning and verification. |
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs |
Expands browser E2E coverage. |
.github/skills/ghpmv-e2e-validation/SKILL.md |
Adds field-sum validation and cleanup flow. |
docs/BROWSER_AUTOMATION_PLAN.md |
Documents automation behavior. |
docs/MANUAL_TEST_PLAN.md |
Adds repeatable manual validation. |
docs/MIGRATION_SCOPE.md |
Records expanded support. |
docs/ui-maps/projects-ui-discovery.md |
Documents discovered UI behavior. |
Suppressed comments (1)
.github/skills/ghpmv-e2e-validation/SKILL.md:951
- The target fallback also reuses an existing empty repository. Recording that repository as
createdmakes the later consented cleanup delete a resource this run did not create; preserve it aspre-existingwhen the fallback is selected.
fixture seed 成功後、出力された target seed Project title / number / URL と `<target-org>/<target-repo>` を resource inventory に `created` として追加する。import 先 Project とは別 entry にする。
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review identified cases where missing browser UI data and reused fallback repositories could be treated as successful or cleanup-owned. Require captured UI settings, preserve fallback repositories, and keep the documented tab order aligned with the fixture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Ghpmv.Core/Browser/ViewUiImporter.cs:774
- Disabled entries are added to
availablebefore this check, so a requested value that is present but disabled is treated as successfully applicable even though it is never clicked. The import then completes without a warning while leaving the target drifted; the inverse case (a disabled checked value that must be cleared) is also silent. Track entries whose current state cannot be changed separately and emit a property-specific warning when that state differs fromdesired.
available.Add(name);
if (string.Equals(await checkbox.GetAttributeAsync("aria-disabled").ConfigureAwait(false), "true", StringComparison.Ordinal))
{
continue;
docs/MANUAL_TEST_PLAN.md:375
- The manual fallback cannot recreate the fixture contract required by the new snapshot gate: it still describes only three tabs, gives the Table only a generic single Number sum, omits the Board sum, and never creates
Fixture Empty Sums. Ifsetup --fixture-uifails and a user follows this path, Step 6 will always reject the fixture. Document the same four named views and exact selections used byFixtureUiSnapshotFactory.
- group by Status
- Field sum に `Fixture Number 2` を設定
A disabled checkbox can be present yet unable to reach the requested state, so availability alone is not enough. Surface that mismatch explicitly and keep the manual fallback aligned with the exact four-view fixture contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
A separate manual scenario duplicated export, import, verify, evidence, and cleanup guidance. Reuse the standard fixture and existing round trip, combining drift repair with the existing idempotence rerun so validation stays complete with fewer commands. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
Future issue plans should extend the shared browser E2E run instead of duplicating full round trips. Define where fixture, snapshot, drift, repair, evidence, and cleanup checks belong, while allowing extra resources only for genuinely distinct code paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/skills/ghpmv-e2e-validation/SKILL.md:213
- A partially successful command is recorded and then the flow stops, but cleanup consent is only reached after a successful Step 10/repair. Failed fixture/import runs can therefore leave newly created Projects or repositories behind without ever offering the inventory-based cleanup path. Route failures after resource creation to cleanup consent (or an explicit retain decision) before ending the run.
追加の disposable target または native command は、fresh/existing、REST/browser、権限境界、destructive preview など、既存 command では別 code path を証明できない場合だけ許可する。追加理由と検証対象を明記し、resource inventory と cleanup に含める。同じ snapshot、mapping、target、verify command を再利用できる場合は複製しない。
.github/skills/ghpmv-e2e-validation/SKILL.md:219
- This cleanup branch requires deletion and read-back but does not provide the concrete Project/repository delete and verification commands; the manual plan only provides
gh repo deleteand says Projects may be deleted via UI or GraphQL. That leaves the agent to invent destructive operations, contrary to this skill's own “実行するコマンドを省略しない” rule at line 25. Add token/host-scoped commands and read-back checks for each inventory resource type.
|---|---|
.github/skills/ghpmv-e2e-validation/SKILL.md:1204
- Remove any existing drift report before running verify. Because a nonzero verify exit is expected here, an authentication/network failure on a rerun can otherwise be combined with a stale prior mismatch report and emit
GHPMV_FIELD_SUM_DRIFT_DETECTEDeven though the current verify did not produce that report.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/ui-maps/projects-ui-discovery.md:98
- This still says Field sum values can be read from the parent item without opening the submenu, but the newly documented
1 moretruncation and the exporter change show that this loses selections. Describe reading all checked child entries instead so this UI contract does not contradict lines 36 and the implementation.
2. **Field sum は Board と grouped Table / Roadmap 共通のチェックボックスオーバーレイ**(`menuitemcheckbox`: "Count" + 数値フィールド名)。menuitem の accessible name は "Field sum: Count and Fixture Number" のようにラベル結合されるため値はメニューを開かず読める。Count は uncheck 可能。Table / Roadmap では未 grouping の間は項目自体が無い
docs/MANUAL_TEST_PLAN.md:503
- The exporter returns
nullwhen no Field sum checkbox is selected, and snapshot serialization omits null properties. Therefore an exportedFixture Empty Sumsview will not contain the documented literalfieldSum=[]; document the omitted-field representation (or change the exporter to emit an empty list) so this manual assertion matchessnapshot.json.
- `Fixture Empty Sums`: `fieldSum=[]`
Review found that failed runs could bypass cleanup, destructive commands were underspecified, stale drift reports could be reused, and two documentation assertions contradicted export behavior. Route failures through explicit consent, define identity-checked delete/read-back commands, clear drift artifacts, and align the docs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/skills/ghpmv-e2e-validation/SKILL.md:361
- This deletion command cannot work for several supported credential paths. The classic PAT plans in this skill never request GitHub's required
delete_reposcope, and the fine-grained PAT plan for a GEI target does not request repository Administration: write. As a result, cleanup after explicit consent will fail for repositories created on those paths. Provision a cleanup credential/permission before deletion (or add the required permission to the applicable token plans) and preflight it before offering repository cleanup.
gh api @cleanupHostArguments --method DELETE 'repos/<owner>/<repository>'
.github/skills/ghpmv-e2e-validation/SKILL.md:1412
- The deliberate-drift checkpoint succeeds whenever the expected message is present, even if the report also contains unrelated View differences, warnings, or errors in other categories. That can mark a broken repair path as
drift-detected, contrary to this flow's requirement to validate the exact intentional drift. Require exactly one non-informational difference—the expectedView 1field-sum mismatch—and reject any additional non-info differences.
$driftViewCategories = @($driftReport.categories | Where-Object category -eq 'View')
$fieldSumDifferences = @($driftReport.differences | Where-Object { $_.category -eq 'View' -and $_.message -match "view 'View 1': field sum mismatch" })
if ($driftViewCategories.Count -ne 1 -or $driftViewCategories[0].status -ne 'Mismatch' -or $fieldSumDifferences.Count -eq 0) {
Repository deletion needs explicit delete authorization that migration credentials do not always provide, and drift verification must reject unrelated failures. Provision or validate a scoped cleanup credential before offering deletion, and require the intentional mismatch to be the only non-info difference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/BROWSER_AUTOMATION_PLAN.md:317
- The updated standard-fixture views reference
Fixture NumberandFixture Number 2, but the field inventory immediately above still documents the oldPriority/Estimate/Start/End/Notesfixture. This makes the specification internally inconsistent withFixtureUiSnapshotFactory.cs:36-56and the new view definitions. Update the inventory to list the actualFixture*fields.
1. "View 1" — grouped Table, filter, sort, Slice by, Field sum=[Count, Fixture Number, Fixture Number 2]
The browser automation plan still described obsolete field names after the fixture evolved. List the actual Status and Fixture fields so the documented view definitions and setup implementation agree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
src/Ghpmv.Core/Browser/ViewUiExporter.cs:186
- This introduces a UI role selector directly in exporter logic, although
Browser/Sel.cs:7-10definesSelas the single source of truth for Projects UI selectors. Move the checkbox/option overlay lookup intoSeland reuse it here (and in the importer) so a future role change cannot make export and import diverge.
var checkboxes = overlay.GetByRole(AriaRole.Menuitemcheckbox);
A null field-sum value could mean either a valid empty selection or an unreadable control, allowing failed capture to clear the target silently. Preserve captured empty selections as an empty list, reject missing controls or entries, and centralize checkable-entry selectors for export and import. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Ghpmv.Core/Browser/ViewUiExporter.cs:181
ReadCheckedMenuValuesAsyncnow throwsInvalidOperationExceptionwhen the expected control or checkable entries are missing, but the per-view catch inEnrichAsync(lines 78–81) only handles Playwright/timeout failures. Either condition therefore aborts the entire export instead of leaving this View's UI uncaptured and emitting the documented warning. IncludeInvalidOperationExceptionin that per-view catch so both throws are handled recoverably.
if (required)
{
throw new InvalidOperationException($"'{label}' control is not available for this grouped view");
src/Ghpmv.Core/Browser/ViewUiImporter.cs:737
- If the child-menu selector returns zero options and the desired selection is empty, the missing-value loop emits no warning, so an unavailable/broken Field sum submenu is incorrectly treated as a successful clear. Since every valid Field sum submenu contains at least
Count, report the empty available set before processing individual missing values.
foreach (var value in values.Where(value => !result.Available.Contains(value)))
{
_warnings.Add($"view '{viewName}': {label} value '{value}' is not available on the target");
}
Expected control failures must remain per-view warnings, and an empty child menu is not a successful clear. Catch capture contract failures at the view boundary and report missing checkable entries during import. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/skills/ghpmv-e2e-validation/SKILL.md:1035
- This gate treats
FieldSumas ordered, although field sums are a selection set (the importer uses aHashSetandProjectVerifier.UiListEqualssorts both sides). An existing equivalent fixture can therefore be rejected solely because GitHub returns the checked fields in a different menu/field order. Compare sorted values while retaining the existing exact membership check.
if ([string]::Join("`0", [string[]]$actualFieldSum) -ne [string]::Join("`0", [string[]]$expected.FieldSum)) {
Stop-FieldSumSnapshotCheck "View '$($expected.Name)' FieldSum mismatch: expected [$($expected.FieldSum -join ', ')], actual [$($actualFieldSum -join ', ')]."
return
GitHub may expose equivalent checked selections in a different menu order. Sort both captured and expected values with ordinal semantics before enforcing exact membership in the E2E snapshot gate. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bee71212-f933-437c-a78a-fb1dfcd5aed7
GitHub Projects exposes field sums only through the web UI, and ghpmv previously replayed them only for Board views. This extends browser-assisted migration so grouped Table and Roadmap views preserve the same complete field-sum selection.
What changed
Count, multiple Number fields, and an empty selection.1 moresummary cannot lose values, and scope checkbox operations to avoid changing unrelated Roadmap display options.ghpmv-e2e-validationbrowser E2E flow.Validation
Fixes: #62