Preserve GitHub Project View tab order - #60
Conversation
GitHub's public APIs expose View positions but cannot update them, so persist and verify POSITION order through GraphQL and use opt-in browser automation for minimal drag-and-drop remediation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Preserves GitHub Project View tab ordering through GraphQL snapshots, verification, and opt-in browser reordering.
Changes:
- Adds backward-compatible
tabPositionsnapshot support. - Imports and verifies Views by tab position.
- Adds LIS-based browser drag planning, fixtures, tests, and documentation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/Ghpmv.Core.Tests/SnapshotTests.cs |
Tests snapshot compatibility. |
tests/Ghpmv.Core.Tests/ProjectViewImporterTests.cs |
Tests position-aware import. |
tests/Ghpmv.Core.Tests/ProjectVerifierTests.cs |
Tests order verification. |
tests/Ghpmv.Core.Tests/ProjectExporterTests.cs |
Tests POSITION export. |
tests/Ghpmv.Browser.Tests/ViewUiLogicTests.cs |
Tests drag planning. |
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs |
Extends browser round-trip coverage. |
src/Ghpmv.Core/Verify/ProjectVerifier.cs |
Compares View order. |
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs |
Adds nullable tab position. |
src/Ghpmv.Core/Import/ProjectViewImporter.cs |
Orders imports and warns on mismatches. |
src/Ghpmv.Core/Export/ProjectExporter.cs |
Exports POSITION order. |
src/Ghpmv.Core/Browser/ViewUiImporter.cs |
Reorders tabs through Playwright. |
src/Ghpmv.Core/Browser/Sel.cs |
Adds draggable-tab selector. |
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs |
Defines fixture tab order. |
src/Ghpmv.Cli/Program.cs |
Supplies GraphQL clients to browser importers. |
docs/ui-maps/projects-ui-discovery.md |
Documents tab drag behavior. |
docs/MIGRATION_SCOPE.md |
Marks browser-assisted order as supported. |
docs/MANUAL_TEST_PLAN.md |
Adds tab-order validation steps. |
docs/BROWSER_AUTOMATION_PLAN.md |
Documents ordering architecture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review identified that overflow dragging and recoverable failures were only partially exercised, so expand the real browser scenario and test both warning layers through the production move executor. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Ghpmv.Core/Export/ProjectExporter.cs:755
- This metadata query still truncates the
viewsconnection at 50, so a project with additional tabs silently omits them and assigns positions only to the first page. The connection has cursors, and the importer already paginates it inProjectViewImporter.cs:429-479; export/verify should likewise fetch all POSITION-ordered pages before assigningTabPosition.
views(first: 50, orderBy: { field: POSITION, direction: ASC }) {
src/Ghpmv.Core/Browser/ViewUiImporter.cs:925
- This read-back only sees the first 50 target tabs. On an updated project with enough pre-existing views, mapped source tabs can fall on a later page, causing the new reorder path to report an incomplete target and skip remediation even though all views exist. Paginate this POSITION-ordered connection, as
ProjectViewImporter.FetchViewsAsyncalready does.
views(first: 50, orderBy: { field: POSITION, direction: ASC }) {
POSITION order must remain complete beyond the first 50 tabs, so continue both snapshot export and browser read-back through every GraphQL cursor page. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
…tab-order # Conflicts: # docs/MANUAL_TEST_PLAN.md # src/Ghpmv.Core/Export/ProjectExporter.cs # tests/Ghpmv.Core.Tests/ProjectExporterTests.cs
The credentialed Integration suite previously ignored tab positions, so add a real-API round trip that proves POSITION export and creation order while confirming API-only updates warn and verify mismatches instead of claiming to repair them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
Name-only tab sequences hide swaps between duplicate-name Views, so use the existing API-visible settings equivalence when duplicate groups are otherwise unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Ghpmv.Core/Verify/ProjectVerifier.cs:782
- Browser-assisted verification still misses swaps between duplicate-name tabs whose API settings are identical but whose UI-only settings differ. The name sequences match, this positional check uses only
ViewApiEquals, and the later UI comparison treats settings as an unordered multiset, so swapping tabs distinguished by Slice by, Field sum, or Roadmap settings produces no difference. Use combined API/UI identity when both sides have UI snapshots, while retaining API-only fallback when browser data is unavailable.
var sourcePositions = sourceOrder.Where(view => string.Equals(view.Name, name, StringComparison.Ordinal));
var targetPositions = targetOrder.Where(view => string.Equals(view.Name, name, StringComparison.Ordinal));
if (!sourcePositions.Zip(targetPositions).All(pair => ViewApiEquals(pair.First, pair.Second)))
Browser-assisted verification can distinguish same-name tabs that differ only in UI settings, so include those settings in positional identity when both snapshots contain them. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
Pull request relinking, production capability preflight, and exporter pagination previously lacked end-to-end coverage against GitHub, so add owned live fixtures and assert their complete remote outcomes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
Linux can interpret a root-relative href as a file URI, so only HTTP(S) links use absolute URI parsing before extracting the View number. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Ghpmv.Core/Browser/ViewUiImporter.cs:164
- The tab-order phase has no recoverable exception boundary. Both its initial DOM read and final readback can throw
PlaywrightException,TimeoutException, orInvalidOperationException; those exceptions currently escapeEnrichAsyncand abort the entire import instead of producing the warning promised for recoverable browser failures. Wrap this phase consistently with the per-view browser operations.
await ReorderTabsAsync(
page,
snapshot,
viewNumbers,
cancellationToken).ConfigureAwait(false);
src/Ghpmv.Core/Browser/ViewUiImporter.cs:37
- This new public overload accepts a
GitHubGraphQLClientbut only null-checks it; the client has no effect and both constructors behave identically. That unnecessarily expands the public API and misleadingly implies GraphQL participates in browser View import. Remove the overload until the dependency is actually needed.
public ViewUiImporter(BrowserSession session, GitHubGraphQLClient client)
: this(session)
{
ArgumentNullException.ThrowIfNull(client);
}
Other browser-only View differences can make the category mismatch, so assert the tab-order warning and absence of a tab-order error directly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
DOM reads can fail before or after dragging, so convert those browser-stage failures to warnings after the API import rather than aborting the migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Ghpmv.Core/Browser/ViewUiImporter.cs:164
- The initial DOM read and the final read-back both run outside the recoverable exception handling in
ApplyTabMovesAsync. If either selector/read fails withPlaywrightException,TimeoutException, orInvalidOperationException, browser-assisted import aborts instead of emitting the warning promised for recoverable tab-order failures. Catch those failures around the reorder stage so API/import work remains usable and the failure is reported.
await ApplyTabOrderRecoverablyAsync(
() => ReorderTabsAsync(
page,
snapshot,
viewNumbers,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/Ghpmv.Core.Tests/ProjectVerifierTests.cs:556
- This test name still attributes the order to GraphQL, but
ProjectExporternow always leavesTabPositionnull and only browser enrichment populates it. Rename the test so it describes comparing captured positions rather than implying API-only GraphQL verification.
public void Graphql_view_position_order_is_compared_without_browser_automation()
docs/MANUAL_TEST_PLAN.md:702
- This expectation says an explicit
tabPosition: nullis saved, but snapshot serialization omits null properties (SnapshotJsonContext.cs:9). State that the property is omitted and interpreted as null so manual validation matches the actual file format.
| N-1 | `--enable-browser-automation` なしで export/import | API-only exportは`tabPosition: null`を保存する。browserで取得済みのtab orderを持つsnapshotのAPI-only importは未適用warning、API-only verifyは`NotVerified`として扱う。Views / Workflows UI-only項目もwarningまたは未移行として扱われる。 |
tests/Ghpmv.Integration.Tests/ItemImporterTests.cs:343
- The test keeps an identity repository mapping and therefore links the canonical source pull request into a project owned by the target organization; it does not relink to a target repository. Rename it to avoid claiming behavior the test does not exercise.
public async Task Pull_request_item_is_relinked_to_the_target_repository_with_its_number_preserved()
The tests cover captured tab positions and identity-mapped PR imports, so align their names and manual snapshot expectations with those exact semantics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Ghpmv.Core/Browser/ViewUiImporter.cs:37
- This public overload accepts a
GitHubGraphQLClientbut never stores or uses it, so it behaves exactly like the one-argument constructor while implying that the client affects tab reordering. Remove the overload to avoid exposing a misleading API surface (or give the client a concrete responsibility).
public ViewUiImporter(BrowserSession session, GitHubGraphQLClient client)
: this(session)
{
ArgumentNullException.ThrowIfNull(client);
}
Tab ordering now reads browser DOM state, so a constructor that accepts an unused GraphQL client misrepresents the importer's dependencies. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
Compare API POSITION order with the saved-tab DOM order during browser export and fail the browser E2E canary when GitHub makes the public read path equivalent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
A newly created target can coincidentally align API and DOM order, so avoid a global recovery notice and let the nontrivial source E2E fixture signal a real API behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62fbc8b4-f2d5-46be-be4d-3feaa5d27afd
GitHub exposes a
POSITIONordering for Project Views, but live validation showed that it can remain different from the saved tab strip shown by GitHub. This change preserves source tab order through opt-in browser automation and uses the public API for the remaining View state.Approach
tabPositionuncaptured because GraphQLPOSITIONis not a reliable saved-tab order.hrefvalues in DOM order and store a nullable zero-basedtabPositionwithout changing schema version 1.Legacy snapshots without
tabPositionremain loadable and skip order remediation.Credentialed API validation
Follow-up validation requested during this PR also closes existing live-API gaps for pull-request item relinking, production import capability preflight, and connection-specific ProjectExporter pagination. PR relinking uses the canonical source fixture repository read-only and owns only its disposable target Project, so the existing credential contract remains unchanged.
Closes #50