Preserve Board column limits - #79
Conversation
GitHub does not expose per-column Board limits through its public APIs, so migrations previously lost this view-specific state. Capture and replay limits through browser automation using logical field/value identities, including clearing, verification, fixtures, and operator guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
The rendering command currently cannot resolve either Board, and duplicate-view verification can miss unavailable limit captures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds browser-assisted migration of Board column limits using logical field/value identities.
Changes:
- Adds snapshot, export, import, and verification support for column limits.
- Extends fixtures, browser E2E validation, drift repair, and documentation.
- Preserves API-only and legacy snapshot behavior.
File summaries
| File | Description |
|---|---|
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs |
Defines the limit snapshot model. |
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs |
Reads, applies, and clears limits. |
src/Ghpmv.Core/Browser/BoardColumnLimitObserver.cs |
Validates rendered fixture limits. |
src/Ghpmv.Core/Browser/ViewUiExporter.cs |
Captures limits during export. |
src/Ghpmv.Core/Browser/ViewUiImporter.cs |
Validates and imports complete limit sets. |
src/Ghpmv.Core/Browser/Sel.cs |
Centralizes Board-limit selectors. |
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs |
Adds Board fixtures and drift state. |
src/Ghpmv.Core/Verify/ProjectVerifier.cs |
Compares captured limits. |
src/Ghpmv.Cli/Program.cs |
Integrates rendering and drift checks. |
tests/Ghpmv.Core.Tests/ProjectVerifierTests.cs |
Tests verification semantics. |
tests/Ghpmv.Core.Tests/FixtureProjectBuilderTests.cs |
Checks over-limit fixture counts. |
tests/Ghpmv.Browser.Tests/ViewUiSnapshotSerializationTests.cs |
Tests serialization compatibility. |
tests/Ghpmv.Browser.Tests/ViewUiLogicTests.cs |
Tests parsing, validation, and drift. |
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs |
Extends shared browser E2E coverage. |
docs/ui-maps/projects-ui-discovery.md |
Documents the UI contract. |
docs/MIGRATION_SCOPE.md |
Records migration support. |
docs/MANUAL_TEST_PLAN.md |
Adds manual validation checkpoints. |
docs/BROWSER_AUTOMATION_PLAN.md |
Documents browser-only architecture. |
.github/skills/ghpmv-e2e-validation/SKILL.md |
Extends the shared E2E workflow. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Rendering checks must resolve both fixture Boards, fixture snapshots must reject stale extra limits, and duplicate Views must not hide unavailable target captures behind a mismatched captured View. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
Missing-target behavior lacks the required coverage, and two fixture-contract documents remain inconsistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Balanced
A missing desired target column must not cause complete-set reconciliation to clear another displayed column. Build a deterministic reconciliation plan that suppresses all writes for the View when any desired column is unavailable, and keep the six-View fixture documentation consistent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
Reconciliation can write incorrect limits after prerequisite or identity validation failures, and duplicate-View verification is order-dependent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs:80
- This plan does not fail closed for malformed captured identities even though preflight only records warnings. A wrong
FieldNameis filtered out here and then every displayed limit is cleared; an Iteration identity on a Single-select field (or an entry with both identities) is keyed by its text and can update that same-named target column. Validate every desired entry against the resolved field and return no targets on any warning before reconciling the complete set.
var desiredByName = desiredLimits
.Where(limit => string.Equals(limit.FieldName, field.Name, StringComparison.Ordinal))
.ToDictionary(GetValueName, StringComparer.Ordinal);
- Files reviewed: 19/19 changed files
- Comments generated: 2
- Review effort level: Balanced
Board limits must not be written after column selection or identity validation fails, and legacy wildcard snapshots must not make duplicate-View matching depend on enumeration order. Gate writes on prerequisites, validate runtime identities, and use bipartite multiset matching. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🔵 Needs a closer look
The credential-gated browser E2E was not run, leaving the new UI-dependent selectors and persistence behavior unverified against GitHub.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
The live Projects UI exposes column actions and limit dialogs differently from the earlier discovery contract, and tab reordering requires pointer activation. Align selectors, dragging, and render observation with the verified DOM so browser-assisted migrations preserve Board limits reliably. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
Snapshot gates, drift assertions, duplicate-set verification, and narrow-width tab dragging have unresolved correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
.github/skills/ghpmv-e2e-validation/SKILL.md:1884
- Counting any two Board-limit differences does not prove the deliberate drift identified Alpha as changed from 1 to 5 and Beta as cleared. Two mismatches for different columns or values would still emit
GHPMV_BOARD_LIMIT_DRIFT_DETECTED, contrary to the workflow's exact-column checkpoint. Filter against the two expected full messages before applying the count check.
$boardLimitDifferences = @($driftReport.differences | Where-Object { $_.category -eq 'View' -and $_.message -match "view 'Fixture Board': Board limit mismatch" })
.github/skills/ghpmv-e2e-validation/SKILL.md:1135
- This snapshot gate can still mark the Board-limit contract as matched when the Board uses the wrong
verticalGroupByFieldsvalue or when the declared unlimited values (Gamma,Sprint 2,Sprint 3) do not exist in the corresponding field. In those cases the later observer/import fails only after target preparation, despite this step being the pre-target hard gate. Validate each Board's single column-by field and the complete Single-select/Iteration value set before emittingGHPMV_BOARD_LIMIT_SNAPSHOT_MATCH.
$actualLimits = @($views[0].ui.boardColumnLimits)
if ($actualLimits.Count -ne $expectedView.Count) {
Stop-FieldSumSnapshotCheck "View '$($expectedView.Name)' boardColumnLimits must contain exactly $($expectedView.Count) configured limits; found $($actualLimits.Count)."
return
}
src/Ghpmv.Core/Verify/ProjectVerifier.cs:1259
- This per-entry lookup is not one-to-one, so duplicate logical entries can falsely verify as a complete-set match: source
[Alpha=1, Alpha=1]versus target[Alpha=1]reuses the same target for both source entries, and the inverse also passes the second loop. Since duplicate identities are explicitly treated as malformed during import, consume matched target entries (or use the existing multiset matcher) so verify cannot reportMatchfor unequal captured sets.
foreach (var sourceLimit in source)
{
var targetLimit = target.FirstOrDefault(candidate => SameBoardColumn(sourceLimit, candidate));
if (targetLimit is null)
- Files reviewed: 19/19 changed files
- Comments generated: 2
- Review effort level: Balanced
A fresh browser import temporarily exposes hidden fields so GitHub can select them as sort keys. Hide only fields introduced by that workaround so the first import preserves the snapshot's visible-field contract instead of requiring a second convergence pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🔵 Needs a closer look
The Step 6 snapshot gate can accept fixtures that omit the required unlimited columns.
Review details
Suppressed comments (1)
.github/skills/ghpmv-e2e-validation/SKILL.md:1123
- The Step 6 gate validates only the four configured entries. It never proves that
GammaandSprint 2/3exist as unlimited columns, so an existing fixture containing only Alpha/Beta and Sprint 0/1 still emitsGHPMV_BOARD_LIMIT_SNAPSHOT_MATCHand proceeds to target creation despite violating the declared fixture contract. Validate the Board column fields' option/iteration sets (including these unlimited identities) before emitting the marker.
$expectedBoardLimits = @(
[pscustomobject]@{ View = 'Fixture Board'; Field = 'Fixture Select'; Property = 'singleSelectOptionName'; Value = 'Alpha'; Limit = 1 },
[pscustomobject]@{ View = 'Fixture Board'; Field = 'Fixture Select'; Property = 'singleSelectOptionName'; Value = 'Beta'; Limit = 2 },
[pscustomobject]@{ View = 'Fixture Iteration Board'; Field = 'Fixture Sprint'; Property = 'iterationTitle'; Value = 'Sprint 0'; Limit = 1 },
[pscustomobject]@{ View = 'Fixture Iteration Board'; Field = 'Fixture Sprint'; Property = 'iterationTitle'; Value = 'Sprint 1'; Limit = 3 }
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Starting a drag only after scrolling both endpoints can move the source outside a narrow tab strip. Activate the source first, then scroll the destination while the drag is active, and align the documented Board menu label with the verified UI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
Hidden Board columns can be omitted or left stale while complete-set capture and reconciliation report success.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs:151
- Complete-set reconciliation only creates targets for displayed columns. If a target column is hidden and the source expects it to be unlimited, it is absent from both
desiredByNameanddisplayedColumnNames, so its stale target limit is neither cleared nor warned about; the subsequent read-back omits it as well and can report success. Reconcile all logical columns, or fail closed when any target column is hidden/unavailable.
var targets = displayedColumnNames
.Where(columnName => ValueExists(field, columnName))
.Select(columnName => new ReconciliationTarget(
columnName,
desiredByName.TryGetValue(columnName, out var configured)
? configured.Limit
: null))
- Files reviewed: 19/19 changed files
- Comments generated: 2
- Review effort level: Balanced
Board limit snapshots are complete sets, so reading or reconciling only rendered columns can silently lose limits. Require every logical option or iteration to be displayed, and remove the stale counter contract that the live swimlane UI does not render. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
The new per-column menu loops omit the repository’s required UI-operation pacing, risking browser automation races.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs:305
- This write path likewise skips the documented 300 ms delay between consecutive UI interactions (
docs/BROWSER_AUTOMATION_PLAN.md:116). Since reconciliation visits every column serially, immediate action/menu/fill/save operations can race GitHub's overlays and violate the repository's UI pacing policy; apply the standard delay throughout this sequence.
await Sel.BoardColumnActionsButton(page, columnName).ClickAsync().ConfigureAwait(false);
var item = Sel.BoardColumnLimitMenuItem(page);
await item.WaitForAsync().ConfigureAwait(false);
await item.ClickAsync().ConfigureAwait(false);
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
GitHub's overlays can remain transitional after visibility checks resolve, so honor the browser automation pacing contract between each limit read and write interaction. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🔵 Needs a closer look
The manual procedure and UI discovery guidance remain inconsistent with the newly documented drift and sort-field behavior.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
docs/MANUAL_TEST_PLAN.md:708
- The new checklist requires Board-limit drift and repair, but the numbered procedure above still tells the operator to expect only seven mismatches (defaults, field sum, and Roadmap dates) and says the re-import restores only Field sum/Roadmap state. A correct run now produces two additional Board-limit mismatches, so following that procedure rejects the expected report and never records Board-limit repair. Update steps 8–11 to include both Board mismatches and Board-limit restoration.
- [ ] targetで`Fixture Board`のAlpha上限を変更しBeta上限をclearすると、verifyが両logical columnを区別して検出する。
- [ ] 同じsnapshot/targetへ再importすると上限が復元され、Item categoryには移動・field value変更がない。
src/Ghpmv.Core/Browser/ViewUiImporter.cs:1628
- This newly unchecks a temporarily shown sort field, but
docs/ui-maps/projects-ui-discovery.md:140still documents the opposite importer contract: the sort field is retained in the desired set specifically to prevent unchecking it. Since the test makes this behavior intentional, update the UI discovery record to explain that hiding the virtual column preserves the sort; otherwise future selector work will follow contradictory guidance.
return sortFieldWasTemporarilyShown
&& !view.VisibleFields.Contains(sortField, StringComparer.Ordinal);
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
The manual drift sequence and UI discovery notes still described behavior from before Board limit drift and sort-field restoration were added, which could make correct validation results look erroneous. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🟡 Changes recommended
The Step 6 snapshot gate can accept fixtures missing unlimited columns or the required over-limit item state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
.github/skills/ghpmv-e2e-validation/SKILL.md:1123
- The Step 6 gate validates only the four configured entries. An existing fixture with no
Gamma/Sprint 2/Sprint 3values, or with only one item in each limit-1 column, still emitsGHPMV_BOARD_LIMIT_SNAPSHOT_MATCH; the missing unlimited and over-limit prerequisites are discovered only after target resources have been created. Extend this early gate to assert those logical values exist and that the snapshot has more than one item assigned toAlphaandSprint 0.
$expectedBoardLimits = @(
[pscustomobject]@{ View = 'Fixture Board'; Field = 'Fixture Select'; Property = 'singleSelectOptionName'; Value = 'Alpha'; Limit = 1 },
[pscustomobject]@{ View = 'Fixture Board'; Field = 'Fixture Select'; Property = 'singleSelectOptionName'; Value = 'Beta'; Limit = 2 },
[pscustomobject]@{ View = 'Fixture Iteration Board'; Field = 'Fixture Sprint'; Property = 'iterationTitle'; Value = 'Sprint 0'; Limit = 1 },
[pscustomobject]@{ View = 'Fixture Iteration Board'; Field = 'Fixture Sprint'; Property = 'iterationTitle'; Value = 'Sprint 1'; Limit = 3 }
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
The snapshot gate previously validated configured limits only, allowing missing unlimited columns and over-limit card placement to fail after target creation. The checklist also referenced a counter that swimlane Boards do not render. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e89ed2d8-70f0-4a29-ae59-2d644c0ac4c6
There was a problem hiding this comment.
🔵 Needs a closer look
The credential-gated live browser E2E was not run, leaving the new undocumented UI interactions without final environment validation.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
GitHub Projects does not expose per-column Board limits through its public APIs, so migrations previously lost this view-specific state. This adds browser-assisted round-trip support while keeping API-only and legacy snapshots non-destructive.
The deterministic Release build and Core/Browser test suites pass. Live Browser E2E remains credential-gated and was not run because the required browser state and test token were unavailable.
Fixes: #66