Preserve Roadmap title and date display settings - #78
Conversation
Capture and replay the two browser-only Roadmap checkboxes so migrations do not silently change title and date rendering. Extend drift and rendered-output checks because checkbox equality alone cannot prove the target view behaves correctly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
The project is still under development, so preserving incomplete schema-v1 snapshots and old fixture operation keys only obscures the current contract. Require current collections and template state, reject mismatched fixture logs, and keep one browser base URL option. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
Older schema-v1 snapshots can be applied destructively, while the Roadmap E2E has correlated-state and false-positive rendering gaps.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds browser-assisted preservation and verification of Roadmap title truncation and date display settings.
Changes:
- Captures, imports, verifies, and deliberately drifts both Roadmap display options.
- Extends fixtures, browser tests, documentation, and E2E guidance.
- Removes legacy snapshot, fixture-log, and setup URL compatibility paths.
File summaries
| File | Description |
|---|---|
tests/Ghpmv.Integration.Tests/VerifyTests.cs |
Adapts verification to concrete snapshot properties. |
tests/Ghpmv.Integration.Tests/ProjectTemplateIntegrationTests.cs |
Removes nullable-template compatibility coverage. |
tests/Ghpmv.Integration.Tests/ProjectExporterTests.cs |
Uses the long Roadmap fixture title. |
tests/Ghpmv.Integration.Tests/ItemImporterTests.cs |
Updates draft-title assertions. |
tests/Ghpmv.Integration.Tests/IntegrationFixtureSnapshotTests.cs |
Updates canonical fixture data. |
tests/Ghpmv.Integration.Tests/IntegrationFixtureSnapshot.cs |
Selects the renamed fixture draft. |
tests/Ghpmv.Core.Tests/StatusUpdateImporterLogicTests.cs |
Removes nullable-history compatibility tests. |
tests/Ghpmv.Core.Tests/SnapshotTests.cs |
Removes incomplete schema-v1 coverage. |
tests/Ghpmv.Core.Tests/ProjectVerifierTests.cs |
Removes legacy-null verification paths. |
tests/Ghpmv.Core.Tests/FixtureProjectBuilderTests.cs |
Updates fixtures and removes legacy-log tests. |
tests/Ghpmv.Core.Tests/CliImportTests.cs |
Updates template-state orchestration tests. |
tests/Ghpmv.Browser.Tests/ViewUiSnapshotSerializationTests.cs |
Tests Roadmap boolean serialization. |
tests/Ghpmv.Browser.Tests/ViewUiLogicTests.cs |
Tests persistence, drift, and verification logic. |
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs |
Adds Roadmap display drift to shared E2E. |
src/Ghpmv.Core/Verify/ProjectVerifier.cs |
Compares concrete collections and Roadmap options. |
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs |
Adds display booleans and concrete contract fields. |
src/Ghpmv.Core/Import/StatusUpdateImporter.cs |
Removes legacy-null handling. |
src/Ghpmv.Core/Import/ProjectTemplateWriteSession.cs |
Requires a concrete final template state. |
src/Ghpmv.Core/Import/ImportCapabilityAnalyzer.cs |
Uses concrete linked repositories. |
src/Ghpmv.Core/Fixtures/FixtureProjectBuilder.cs |
Adds long-title fixture and removes legacy paths. |
src/Ghpmv.Core/Export/ProjectExporter.cs |
Emits concrete collections and current View schema. |
src/Ghpmv.Core/Export/MappingTemplates.cs |
Uses concrete linked collections. |
src/Ghpmv.Core/Browser/ViewUiImporter.cs |
Applies and reload-verifies both controls. |
src/Ghpmv.Core/Browser/ViewUiExporter.cs |
Reads both direct menu checkboxes. |
src/Ghpmv.Core/Browser/Sel.cs |
Adds the direct checkbox selector. |
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs |
Adds standard and drift display states. |
src/Ghpmv.Core/Browser/FieldSumRenderingObserver.cs |
Adds rendered Roadmap checks. |
src/Ghpmv.Cli/Program.cs |
Adds the drift command and removes URL aliasing. |
README.md |
Removes legacy template behavior documentation. |
docs/ui-maps/projects-ui-discovery.md |
Records discovered checkbox behavior. |
docs/ui-maps/insights-ui-discovery.md |
Removes obsolete compatibility wording. |
docs/MIGRATION_SCOPE.md |
Documents Roadmap display migration coverage. |
docs/MANUAL_TEST_PLAN.md |
Adds display, drift, and repair checkpoints. |
docs/BROWSER_AUTOMATION_PLAN.md |
Extends the Roadmap UI settings scope. |
.github/skills/ghpmv-e2e-validation/SKILL.md |
Integrates Roadmap checks into shared E2E. |
Review details
- Files reviewed: 35/35 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Reject obsolete snapshot schemas before import, exercise title truncation independently from date display, and scope rendered-date checks to the matching Roadmap item. Keep the long rendering fixture isolated to newly created E2E resources so the shared integration fixture remains stable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
Unavailable controls currently discard all Roadmap UI capture, and standard fixture paths do not consistently create the required rendering item.
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.
docs/ui-maps/projects-ui-discovery.md:41
- This records the selector and intended read-back mechanism, but not the discovery gate's actual outcomes: whether each value persisted after save/reload, whether it is per-view or shared, and whether menu text exposes the current value. Issue #65 requires those live observations to be recorded before the schema change; add the dated evidence for each control (or mark an unsupported control accordingly).
8. `Truncate titles` / `Show date fields` は親 View menu の direct `menuitemcheckbox` で、状態は `aria-checked` に保持される。ghpmv は子 menu の `Markers` / `Field sum` と分離して読み書きし、Save view → reload 後に各 boolean を再読する
- Files reviewed: 32/32 changed files
- Comments generated: 3
- Review effort level: Balanced
Keep readable Roadmap settings when one undocumented checkbox is unavailable, and require a complete long dated rendering item for browser fixture workflows. Align the drift contract and discovery notes with the independently verified title-only transition. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
Schema-v2 template presence is not enforced, and the required live persistence discovery remains incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Snapshot/ProjectSnapshot.cs:115
- Schema-v2 files are accepted based only on
schemaVersion, but this non-required property defaults tofalsewhenproject.templateis omitted. The CLI now always applies that value, so a malformed schema-v2 snapshot can silently unmark an existing template. Make the property required (updating snapshot constructors/tests) or validate its JSON presence before any import writes.
public bool Template { get; init; }
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Balanced
Reject malformed schema-v2 snapshots that omit project.template before deserialization reaches any import write path. This keeps removal of the legacy nullable sentinel explicit and fail-safe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
The shared browser E2E does not exercise rendered output and does not reject unintended date-display drift.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
tests/Ghpmv.Browser.Tests/BrowserRoundTripTests.cs:307
- This only requires the truncation mismatch; the E2E still passes if applying
(false, true)accidentally also changesShow date fields. Explicitly reject that second mismatch so the test proves the two controls drift independently.
Assert.Contains(driftReport.Differences, difference =>
difference.Severity == VerifySeverity.Error
&& difference.Category == VerifyCategories.View
&& difference.Message.Contains("truncate titles mismatch", StringComparison.Ordinal));
src/Ghpmv.Core/Browser/FieldSumRenderingObserver.cs:64
- The shared
BrowserRoundTripTestsE2E never invokes this new DOM checkpoint; only the standalonesetup --fixture-field-sum-render-checkCLI path calls it. Consequently, the repository's sole automated browser round trip can pass even if actual title truncation or date rendering is broken. InvokeFieldSumRenderingObserver.ValidateStandardFixtureAsyncafter the initial target round trip and before deliberate drift.
if (string.Equals(view.Layout, "ROADMAP_LAYOUT", StringComparison.Ordinal))
{
await ValidateRoadmapDisplayAsync(page, view).ConfigureAwait(false);
- Files reviewed: 32/32 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Run the DOM rendering checkpoint inside the shared browser round trip and explicitly reject unintended date-display drift. This makes independent control behavior part of the repository's primary browser scenario rather than only a standalone CLI check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
The shared browser flow never tests ShowDateFields=false or per-view isolation.
Review details
Suppressed comments (1)
src/Ghpmv.Core/Browser/FixtureUiSnapshotFactory.cs:212
- The shared browser flow never exercises
ShowDateFields = false: the standard fixture and repair usetrue, while the only drift deliberately preservestrue. Consequently, an importer that always enables or ignores this checkbox would still pass the browser round trip, drift, and repair checks; the four-combination theory covers serialization only. Issue #65 also requires mixed per-view combinations, but this fixture contains only one Roadmap, so it cannot prove per-view isolation. Add a second Roadmap with a mixed state such as(true, false)to the same shared E2E flow, assert its rendered/menu state, and verify it remains unchanged when the first Roadmap is drifted.
TruncateTitles = true,
ShowDateFields = true,
- Files reviewed: 32/32 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Add a second Roadmap that keeps title truncation enabled while hiding date fields. Validate both rendered states in the shared browser round trip and ensure drifting the first Roadmap cannot alter the second, proving per-view isolation and the false date-display path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
The manual snapshot checkpoint omits the new values, and new DOM selectors bypass the repository’s selector registry.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Ghpmv.Core/Browser/FieldSumRenderingObserver.cs:154
- This repeats the Roadmap-item selector inside observer logic even though
Sel.cs:8-10definesSelas the single source of truth andSel.RoadmapItemalready owns this UI assumption. If GitHub changes the item container, the locator and this boundary test can diverge; reuse a boundary supplied bySelinstead of embedding the role/test-id/class predicate here.
This issue also appears on line 164 of the same file.
if (node.matches("[role='row'], [role='listitem'], [data-testid*='roadmap-item'], [class*='roadmap-item'], [class*='RoadmapItem']")) {
return false;
src/Ghpmv.Core/Browser/FieldSumRenderingObserver.cs:164
- The inline
time, relative-timeselector bypasses the selector registry, contrary to theSel.cs:8-10single-source-of-truth convention. Move this date-element lookup intoSel(while retaining the item-scoped text fallback) so GitHub UI selector changes remain centralized.
return item.querySelector('time, relative-time') !== null || datePattern.test(item.innerText);
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Balanced
Keep Roadmap item boundaries and date-element selectors in Sel so observer logic cannot drift from the UI map. Expose both display booleans in the manual snapshot checkpoint so null or incorrect capture is visible. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
The rendering check currently treats hidden date elements as visibly rendered.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Balanced
Run the default-branch reporting workflow with PowerShell and make the supported combined fixture setup the primary manual path so Roadmap rendering prerequisites are always seeded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
The write-privileged reporter can publish attacker-controlled Markdown from untrusted TRX test names.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 54/54 changed files
- Comments generated: 1
- Review effort level: Balanced
Treat TRX test names as untrusted input by removing line separators and numeric-entity encoding every non-alphanumeric character before rendering the name in the trusted workflow's sticky comment. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
The 54-file change combines browser-state persistence, live UI selectors, workflow reporting, and a breaking snapshot-contract migration that warrants final human validation.
Review details
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Remove the obsolete pull-request write request so the read-only CI caller can validate and start the reusable workflow; trusted reporting remains isolated in workflow_run. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
It combines a breaking persisted-schema change, browser-storage behavior, live UI automation, and privileged workflow reporting that warrant final human validation.
Review details
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Keep github.token out of the reporting job environment so ghalint accepts the trusted workflow and each API operation receives credentials only where needed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
The E2E skill omits the required warning that these controls depend on undocumented browser UI.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/skills/ghpmv-e2e-validation/SKILL.md:211
- Issue #65 requires the E2E guidance to announce that these Roadmap controls rely on undocumented browser UI, but this new contract presents them like stable supported settings and never tells the operator about that risk before creating resources. Add an explicit pre-fixture warning so users can make an informed decision when GitHub changes or removes the controls.
同じ round-trip でProject-shared Roadmap state `(truncateTitles=true, showDateFields=false)` を両Roadmapからcaptureし、DOM checkpointでtitle truncationとdate非表示を検証する。negative-test phaseはtitle-only `(false,false)` とdate-only `(true,true)` をそれぞれ別checkpointで適用し、exact mismatch、DOM rendering、baseline repairを各段階で確認する。
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Require browser E2E operators to acknowledge that Roadmap display controls and storage are outside GitHub's public compatibility contract before creating fixtures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🔵 Needs a closer look
The live-result reporter can let an older rerun overwrite the sticky status for the PR’s current head.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/live-api-report.yml:36
- A rerun of an older CI run can overwrite or resolve the sticky comment for the current PR head because this reporter never checks that the completed run still belongs to the PR's current
head.sha. Before reading/posting the result, skip runs whoseworkflow_run.head_shano longer matches the pull request head; otherwise the comment can report stale status as the “latest” result.
- name: Find Live GitHub API result
id: result
env:
GH_TOKEN: ${{ github.token }}
run: |
$jobsJson = gh api --paginate --slurp "repos/$env:REPOSITORY/actions/runs/$env:RUN_ID/jobs?per_page=100"
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Key sticky comments by immutable head SHA so an older rerun can only update its own result, even if the PR head changes between validation and the REST mutation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
There was a problem hiding this comment.
🟡 Changes recommended
The SHA-scoped reporter marker leaves stale failure comments after fixes land in newer commits.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 54/54 changed files
- Comments generated: 1
- Review effort level: Balanced
Use one PR-stable marker so newer successes recover prior failures, while serializing reporters without cancellation and retaining head checks so stale reruns cannot interrupt or overwrite the current result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02264cba-82f0-4529-84e3-1f9477685733
GitHub Projects exposes
Truncate titlesandShow date fieldsthrough browser-local Roadmap preferences rather than the Project API. This change captures and restores the project-shared values through browser profiles, flushes browser storage after every View UI write path, and verifies the saved profile state across session boundaries.Summary
Validation
(false,true)drift and(true,false)repairFixes: #65