Preserve Board column visibility during migration - #81
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
A critical verification gap and multiple moderate visibility, fixture, export, and validation issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds browser-assisted capture, replay, and verification of GitHub Project Board column visibility using logical field/value identities.
Changes:
- Preserves Single-select and Iteration Board visibility.
- Verifies persisted visibility and reports drift.
- Extends filter handling, fixtures, tests, documentation, and E2E validation.
File summaries
| File | Review |
|---|---|
tests/Ghpmv.Core.Tests/ProjectViewImporterTests.cs |
Tests custom-field filter import. |
tests/Ghpmv.Core.Tests/ProjectVerifierTests.cs |
Tests visibility drift and uncaptured states. |
tests/Ghpmv.Core.Tests/ProjectFilterTransformerTests.cs |
Tests field-derived qualifiers. |
tests/Ghpmv.Core.Tests/CliImportTests.cs |
Verifies View-only field fetching. |
tests/Ghpmv.Browser.Tests/ViewUiSnapshotSerializationTests.cs |
Covers visibility serialization compatibility. |
tests/Ghpmv.Browser.Tests/ViewUiLogicTests.cs |
Tests visibility reconciliation and comparison. |
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs |
Extends browser round-trip coverage. |
src/Ghpmv.Core/Verify/ProjectVerifier.cs |
Critical (1 vote): Duplicate identities can incorrectly verify as matching. |
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs |
Defines logical column snapshots. |
src/Ghpmv.Core/Import/ProjectViewImporter.cs |
Accepts project-field qualifiers. |
src/Ghpmv.Core/Import/ProjectImporter.cs |
Supplies field metadata for verification. |
src/Ghpmv.Core/Import/ProjectFilterTransformer.cs |
Recognizes field-derived filters. |
src/Ghpmv.Core/Browser/ViewUiImporter.cs |
Applies and verifies visibility. |
src/Ghpmv.Core/Browser/ViewUiExporter.cs |
Moderate (2 votes): Visibility-read failures also suppress independent Board-limit capture. |
src/Ghpmv.Core/Browser/Sel.cs |
Adds the visibility-picker selector. |
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs |
Moderate (1 vote each): Single-select and Iteration fixtures lack required empty hidden columns. |
src/Ghpmv.Core/Browser/BoardColumnVisibilityUi.cs |
Moderate (1 vote): Iteration reconciliation may hide before revealing a replacement, violating the three-column minimum. |
src/Ghpmv.Core/Browser/BoardColumnVisibilityObserver.cs |
Validates rendered visibility. |
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs |
Supports limits on visible columns. |
src/Ghpmv.Cli/Program.cs |
Moderate (1 vote): Renamed completion marker no longer matches the E2E workflow gate. |
docs/ui-maps/projects-ui-discovery.md |
Documents the UI contract. |
docs/MANUAL_TEST_PLAN.md |
Adds visibility checkpoints. |
.github/skills/ghpmv-e2e-validation/SKILL.md |
Nit (1 vote): Success gate expects the obsolete completion marker and omits visibility status. |
Review details
Suppressed comments (1)
.github/skills/ghpmv-e2e-validation/SKILL.md:1672
- The new automated visibility observation is not reflected in the command’s success gate below: it still requires the old
Fixture field-sum and Board-limit rendering verifiedtext, butProgram.csnow emitsFixture field-sum, Board-limit, and Board-visibility rendering verified. Following this skill would reject a successful render check and never advance. Update the expected summary and require the visibility observer output/status too.
このため Group by、Field sum menu、Board列pickerの選択状態、空集合について対話用質問を重ねない。Issue #62 の派生描画 checkpoint とBoard visibilityの直接UI観測も、初回 `View: Match` 後にPlaywrightで自動検証する。ユーザーへbrowser reloadや自己申告を求めない。
- Files reviewed: 23/23 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Hidden-column limits can be lost, and the remaining moderate correctness and verification issues must be resolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Ghpmv.Core/Verify/ProjectVerifier.cs:1019
- For duplicate-named Views, this matches an uncaptured target using API settings only. A target with different Field sum, Roadmap settings, or Board limits can therefore be paired with this source View and incorrectly mark visibility
NotVerified. Match all other UI state while excluding onlyVisibleColumns, as the adjacent Board-limit helper does.
- Files reviewed: 25/25 changed files
- Comments generated: 5
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate visibility migration and validation issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
docs/MANUAL_TEST_PLAN.md:643
- The main round-trip sequence below still describes the old drift: step 8 mentions only field sum/limits, step 9 expects only the pre-visibility mismatches, and steps 10–11 omit visibility repair. Because
--fixture-field-sum-driftnow creates four additional visibility differences, following this manual flow will reject the expected report and does not explicitly verify repair. Update those steps to match the new integrated drift contract.
`Rendered Field sums verified`、`Rendered Board limits verified`、`Rendered Board visibility verified` が対象Viewに出力され、最後に `Fixture field-sum, Board-limit, and Board-visibility rendering verified: project=#<target-project-number>` と exit code 0 になることを確認します。両Boardで数値上限、unlimited列、limit=1を超えるcount、visible/hidden列を機械確認します。
src/Ghpmv.Core/Browser/BoardColumnVisibilityUi.cs:85
- Missing target values are checked only against
plan.VisibleNames, and only after other columns have already been toggled. If a source-hidden iteration is absent/renamed on an existing target field, no warning is emitted; if a source-visible value is absent, this can hide other columns before returning the warning. Compare the picker against every logical source value and abort before any writes when one is missing.
foreach (var change in BuildApplyOrder(available.ToList(), plan.VisibleNames))
{
await ApplyVisibilityAsync(change.Name, change.ShouldBeVisible).ConfigureAwait(false);
}
foreach (var missing in plan.VisibleNames.Where(name => !available.Contains(name)))
{
plan.Warnings.Add(
$"view '{view.Name}': visible Board column '{field.Name}' / '{missing}' is not available on the target; no other column was selected");
src/Ghpmv.Core/Browser/ViewUiImporter.cs:1308
- After the final visibility set is saved and the page is reloaded, this method reads Board limits before visibility.
BoardColumnLimitUi.ReadAsyncnow requires every logical column to be displayed (BoardColumnLimitUi.cs:22-27), so any snapshot that intentionally hides a column while also capturing limits throws here andEnrichAsyncreports the whole View import as a warning. Verify limits by temporarily revealing/restoring all logical columns (as the exporter does) or otherwise separate limit verification from the final hidden state.
if (view.Ui?.VisibleColumns is not null)
{
settings = settings with
{
VisibleColumns = await BoardColumnVisibilityUi.ReadAsync(
page,
view,
fields,
cancellationToken).ConfigureAwait(false),
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate correctness issues remain in complete limit reads and duplicate-view verification.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Ghpmv.Core/Verify/ProjectVerifier.cs:1154
- For duplicate-named views, this rejects a target whose limits are also uncaptured, even though null limits are wildcarded everywhere else. That can yield a false
Match: a source view with captured visibilityX/limitsLcan pair with an all-null target, while a legacy source view pairs with a target whose captured visibility isY/limitsL; the combined multiset passes, and this helper fails to add the required visibilityNotVerifiedwarning. Use the existing null-aware limit comparison here so simultaneous visibility/limit capture failure is detected.
src/Ghpmv.Core/Browser/BoardColumnLimitUi.cs:27
- This completeness check makes both existing
ReadAsynccallers fail on the new fixture:BoardColumnLimitObserver.ValidateFixtureAsyncandViewUiImporter.ApplyBoardColumnLimitsAsyncread Boards where Gamma/Delta and Sprint 2/Sprint 4 are intentionally hidden, soexpectedNamescan never be a subset of the displayed headings. The render-check command and the field-sum drift command therefore emit a warning/exit 1. Keep the complete-read requirement for export, but update these callers to capture current visibility and useReadCompleteAsync(temporarily reveal, read, and restore).
var expectedNames = GetValueNames(field).ToHashSet(StringComparer.Ordinal);
var displayedNames = columns.Select(column => column.Name).ToHashSet(StringComparer.Ordinal);
if (!expectedNames.IsSubsetOf(displayedNames))
{
throw new InvalidOperationException(
$"view '{view.Name}': not all logical Board columns were displayed for complete limit capture");
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The rendered-visibility progress marker must match the documented validation gates.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Ghpmv.Core/Browser/BoardColumnVisibilityObserver.cs:60
- The observer emits
Rendered Board column visibility verified, but the updated validation gates require the exact substringRendered Board visibility verified(.github/skills/ghpmv-e2e-validation/SKILL.md:1724anddocs/MANUAL_TEST_PLAN.md:643). A successful render check therefore never produces the per-Board marker those workflows wait for. Align this progress text with the documented marker (or update both gates consistently).
- Files reviewed: 26/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Three moderate correctness issues remain in limit capture, filter transformation, and duplicate-view verification.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
src/Ghpmv.Core/Browser/ViewUiExporter.cs:190
- When visibility capture fails, this branch skips Board-limit capture entirely. If all logical columns are already displayed,
BoardColumnLimitUi.ReadAsynccan still produce a complete limit set without using the new visibility selector, so anAddBoardColumnButtonselector failure now drops previously exportable limit state. Attempt that independent complete read whenvisibleColumnsis null, and leave limits uncaptured only when its completeness check fails.
src/Ghpmv.Core/Import/ProjectFilterTransformer.cs:226 FieldSnapshotincludes built-in fields, so deriving qualifiers from every field also marks identity-bearing filters such asreviewers:USERNAMEandparent-issue:OWNER/REPO#Nas safe passthrough. Those values then remain source identities and the previous unsupported warning is suppressed, producing an invalid target filter after user/repository mapping. Restrict derived passthrough qualifiers to value-only custom field types; identity-bearing built-ins need their own mapping support.
src/Ghpmv.Core/Verify/ProjectVerifier.cs:1152HasUncapturedTargetBoardLimitsstill pairs duplicate-name views throughViewUiEqualsWithoutBoardLimits, but that helper ignores the newly added visibility state. If only one source duplicate has captured limits and the target duplicate with matching visibility hasBoardColumnLimits = null, the helper can instead match the captured limits on the other visibility variant, so verification can reportMatchwithout the required not-verified warning. Include visibility when matching “all UI except limits,” just as this inverse helper includes limits.
- Files reviewed: 26/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Hidden-column limits and MULTI_SELECT filter qualifiers remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Browser/ViewUiImporter.cs:415
ApplyBoardColumnLimitsAsyncstill writes limits before revealing hidden columns. Since export can now capture a configured limit on a hidden column viaReadCompleteAsync, passing that valid snapshot here makesBoardColumnLimitUi.ApplyAsyncreport the desired column as missing and leave its limit unchanged. Temporarily reveal all logical columns before applying the limits, then restorecurrentVisibilityin afinallyblock, matching the main import path’s reconciliation order.
_warnings.AddRange(await BoardColumnLimitUi.ApplyAsync(
page,
view,
fields,
desired,
cancellationToken).ConfigureAwait(false));
- Files reviewed: 26/26 changed files
- Comments generated: 1
- Review effort level: Balanced
Capture and replay logical Board column visibility because GitHub does not expose this setting through the public Projects API. Keep custom-field filters warning-free and verify the UI state through deterministic and live round-trip coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Apply visible columns before hiding others so GitHub's minimum-column rule cannot block reconciliation, and keep independent UI captures from masking each other. Expand fixtures and verification to cover duplicate identities plus populated and empty hidden columns. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Board limits are only exposed for rendered columns, so temporarily reveal every logical column during complete capture and import reconciliation, then restore the intended visibility. Tighten duplicate-view pairing and drift assertions so incomplete or incorrect UI state cannot pass verification. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Preflight every logical field value before changing visibility so renamed hidden columns cannot produce partial updates. Reuse complete limit capture during post-save verification, and align validation guidance with populated hidden columns and visibility repair. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Hidden columns make displayed-only limit reads incomplete, so render and drift verification now capture current visibility and restore it around complete reads. Preserve null-aware limit matching for duplicate views so simultaneous capture gaps remain NotVerified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Use the exact documented E2E sentinel so successful visibility observation advances the validation workflow instead of appearing incomplete. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Retain complete displayed-column limit capture when visibility selectors fail, but do not derive passthrough qualifiers from identity-bearing built-ins. Pair duplicate-view limit capture gaps using the corresponding visibility state to avoid false matches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
Treat multi-select fields as value-only custom qualifiers while keeping identity-bearing built-ins unmapped. Reveal all Board columns around standalone limit drift so configured hidden limits can be written and verified without changing final visibility. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8043ea3a-7920-423d-9f13-09b503dc3773
db4731e to
dfc0d6d
Compare
Live GitHub API check recovered for
|
GitHub Projects does not expose per-value Board column visibility through the public API, so migrations currently lose which Single-select and Iteration columns are shown. This adds browser-assisted capture, replay, and verification for that UI-only state.
What changed
Validation
views: imported=6 warnings=0; browser-assisted View verification completed withView: Matchand 0 warnings.Closes #64