Skip to content

Remove redundant sidebar framing and round navigation selection - #876

Merged
coneilen merged 2 commits into
mainfrom
coneilen-sidebar-framing-parity
Oct 19, 2026
Merged

coneilen merged 2 commits into
mainfrom
coneilen-sidebar-framing-parity

Conversation

@coneilen

@coneilen coneilen commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Remove the redundant fixed GRAPH/Projects title block and use inset rounded sidebar selection. Repair commit ca68dd07eb0c0c8e18210ce165d25fd904a2891a, normally pushed above ba49b7df6edb7dfc1154e1963065af3c72eb0e59, also updates the eight direct App test/helper contracts that still assumed the old framing; it changes no production behavior.

Changes

  • Keep Graph and Quick Chats adjacent and put the 18-point separation on each present folder caption. The macOS authority is the sidebar-style List and its directly emitted destinations: AppSidebarView.swift:77, 334-369, and caption-owned spacing at 387-418.
  • Reuse rounded selection for Graph, Quick Chats, selected projects and reclaimable worktrees, with existing selection predicates and neutral colour. Blue versus grey is activation-qualified, not unconditional recolouring. The Windows 8px initial inset, symmetric 12px row insets and 6px corner radius are mappings, not numeric SwiftUI constants. Existing row heights remain unchanged.
  • Preserve Align Windows sidebar row presentation with macOS #873's palette, stripes, fonts, measured compound ages, states, icons, indentation, invalid timestamps, routing and selection semantics. Keep Needs-you and all footer/scroll guards. No graph layout, shared tokens, restart/tab/header, fixture, registrar or parity-ledger changes.
  • Repair scope: only eight App inline-test/helper hunks, 20 insertions and 10 deletions, separately declared and authorized while the graph-layout writer owned other App hunks. Sidebar.zig, App production functions, graph/workspace literals, the shared DPI helper, layout refresh and teardown are unchanged by the repair. The source-bound contracts use Sidebar.zig:419-420, 428-461, 656-665, 711-765, and App's minimum-height formula.
  • Why the changed expectations are not coverage reductions: move the wholly hidden-row fixture above the actual content inset; expect the independent row right edge at 208, not 220; populate actual rows beneath the footer before testing refusal; decompose the minimum height as 34 + 8 + 52 + 58 + 250 + 48 = 450 instead of retaining the deleted title reservation. All physical/system-minimum and selection/no-command negatives remain.
  • Add explicit minimum-height 96/144-DPI banner cases rather than deleting collapsed coverage: the original 16 combinations now fit, while the additional cases yield four visible and four collapsed combinations, so 20 offers and four collapses are checked. Derive enough 24px loop slots from three viewport heights while retaining the greater-than-two-pages setup guard and every scroll refusal. The local-caption DPI literal alone becomes 12,156,208,180, once scaled: 156 derives from the 42px initial boundary, three 32px destination/chat slots and the 18px caption gap. All 286 App test declarations remain in order.

Test plan

Repair validation: the retained failed hosted job on ba49 reported 1286 passed, 20 skipped and eight failed in the full 1314-case App root. Exactly those eight failures were reproduced locally on clean ba49 before editing. The following definitions spell out the native arguments used by the private recorded test harness, from graphcode-windows, with pinned Zig 0.15.2 and owned provider/cache/temp directories:

$appArgs = @(
  "test", "src\App.zig", "src\AccessibilityProvider.cpp", "src\FilePicker.c", "src\FolderPicker.c",
  "C:\gc-l8-428a7ff9\host-out\lib\winghostty-win32-host.lib", "-DUNICODE", "-D_UNICODE",
  "-target", "x86_64-windows-msvc", "-lc", "-luser32", "-lgdi32", "-lgdiplus", "-lmsimg32",
  "-lopengl32", "-lkernel32", "-limm32", "-lole32", "-loleaut32", "-luiautomationcore",
  "-lshell32", "-ladvapi32", "-lwinhttp", "-I$env:GRAPHCODE_WINGHOSTTY_ROOT\include",
  "C:\gc-l8-428a7ff9\app-cache\o\9e48a6873c4a1ae3f84821fc9f48e211\terminal-vt\lib\ghostty-vt-static.lib",
  "--cache-dir", "C:\gc-l8-428a7ff9\repair-app-cache"
)
$eightFilters = @(
  "--test-filter", "sidebar UIA bounds clip hidden rows",
  "--test-filter", "parity sidebar navigation UIA matches painted",
  "--test-filter", "sidebar footer is not a click target",
  "--test-filter", "sidebar UIA bounds are the drawn rect",
  "--test-filter", "the minimum window height keeps",
  "--test-filter", "banner click and the UIA dynamic invoke",
  "--test-filter", "UIA scroll requests move",
  "--test-filter", "DPI UIA logical cards headers sidebar"
)

RED: & $env:GRAPHCODE_ZIG0152 @appArgs @eightFilters -> 0 passed; 0 skipped; 8 failed on clean ba49: hidden-row fixture expected empty but became visible; navigation/drawn row right 220 versus 208; no hidden footer row exercised; minimum 517 versus 450; offers 14 versus 16; insufficient greater-than-two-pages scroll fixture; local-caption DPI top 229 versus 156.
GREEN: & $env:GRAPHCODE_ZIG0152 @appArgs @eightFilters -> All 8 tests passed, with the same eight case identities and retained refusal, clipping, selection, offscreen paint and once-only DPI checks.
REGRESSION: & $env:GRAPHCODE_ZIG0152 @appArgs -> 1298 passed; 15 skipped; 1 failed of 1314, solely unchanged WindowsNativeDialogs DPI relayout expected 144, found 96; not a local full-App pass.

  • The unfiltered App root ran twice in distinct actual contexts: the direct command above and the App section of the actual full default Tools\windows\validate.ps1 -Task windows-shell -SwiftExecutable $env:GRAPHCODE_SWIFT633 -ShellValidationRoot C:\gc-l8-428a7ff9\suite-validation invocation. Both reported exactly 1298 passed, 15 skipped, one failed. All eight repaired cases passed in both. The 15 default-path App skips are the three opt-in paint exports and 12 opt-in native-AA/chrome cases; no filter replaced the full root.
  • Full windows-shell is not claimed green. The unchanged default runner invoked all 56 registered sections, with 55 completing before its final App root failed on that same native-dialog case. Registered Sidebar passed 222/222, Navigation passed 2/2, and standalone WindowsNativeDialogs passed 33/33 inside this run. The post-unit native accessibility-provider and integration stages were not reached. Swift daemon/CLI release builds completed; the existing isolation finally verified default profiles unchanged and removed its owned root. There was no capture or foreign-process cleanup experiment.
  • Operator-approved classification/publication exception: the same hosted job at ba49 records this exact dialog test as OK both at standalone case 8/33 (23:44:06Z) and App case 1011/1314 (23:48:12Z). Its source is unchanged by this repair; an isolated local owning-root run also passed 1/1. The operator classified the local 144-versus-96 result as host-local desktop/DPI context and authorized the scoped normal push without another native retry. That is not a proven message chronology or a full local pass; no sibling-interference attribution, guard relaxation, NativeDialogs edit, skip or hidden rerun is claimed. Actual new-head CI is the discriminator.
  • Exactly one fresh default-configuration code-review agent examined only the repair delta against ba49, not the whole PR: 5517eb53-b850-4878-aa6c-76317cb1283a, verdict No significant issues found in the reviewed changes. It confirmed source-derived expectations and meaningful visible/collapsed coverage; it performed no tests or edits. The source hash was rechecked unchanged before committing.
  • git diff --check passed; staged and working App.zig are LF. Whole-file zig fmt --check App.zig fails on unrelated preexisting whitespace in both the exact ba49 baseline and the repaired file; that formatting was not changed across another writer's surfaces. The first full-run setup stopped before tests when its profile snapshot encountered an inaccessible foreign default log. The corrected invocation supplies owned process-only USERPROFILE/LOCALAPPDATA/APPDATA before entering the unchanged wrapper; its profile guards remain intact and both attempts are retained separately.

Current-head CI snapshot: after the owning normal push, the operator's train moved the live draft to 27ac6bb8d81ef930ff4a9654d21e6bcd0ede9011 on main. At 2026-10-19T00:47:06Z its positive inventory had 17 checks: seven successful, two skipped, eight pending and zero failed. This is pending, not terminal green. Local repair tests/review remain bound to the exact source committed as ca68dd07; no owner rebase, extra native retry or new-head runtime/visual claim is inferred from the operator's later head.

History and source bindings: original fork 48153975bb13c8b98367ae51a57656accc8b3159 and original own commit 901dbb290ce2502c3716ca8d2ede07a356de593e remain preserved. After #873's squash e0ae6a6bdf652652e0566cf4b53418ab176ba764, GitHub mechanically replayed the one own commit as ba49b7df6edb7dfc1154e1963065af3c72eb0e59 on main; range-diff was equivalent and Sidebar blob cec626512b4adfcadb357e125a1bd5134a59b5af was identical. The preexisting local dcf84c69 merge was saved as unused unpublished entry history. The clean identical worktree was aligned to live ba49 without publishing that merge, resetting files, rebasing or mixing later main changes. Repair publication is one new signed-off commit ca68dd07eb0c0c8e18210ce165d25fd904a2891a, sole parent ba49, with immediate live-tip guards and a normal fast-forward push; no amend/force/rebase. Native stack 877 retains merged #873 and this draft in order.

Historical original-implementation evidence, not rerun or relabelled: six behavioral framing failures on original 4815 source plus tests became 6/6 focused and 222/222 Sidebar, with Navigation 2/2 and a ReleaseSafe production build. The original one code review and one paired v12 overview visual review remain bound to 901d; the production offscreen export passed 1/1 with three matching memory-DIB renders and one private PNG. Its unchanged converter profile excludes Sidebar, whose hash was bound separately. Those records and the original old-head capture-attestation CI failure are preserved, not upgraded by this test-only repair.

Parity remains Partial. Different fixtures, material, dimensions, loop density and activation states prevent a full-frame match. No new visual review, image upload, live capture, higher-DPI visual acceptance, physical input, daemon persistence, macOS build/test, post-merge acceptance, Ready or merge action is claimed. Private gap documents and images remain private. make test and make check were not run on this Windows host.

Checklist

  • I have read the Contributing Guidelines
  • I have signed off my commits (git commit -s) per the DCO
  • Tests pass locally (make test)
  • Code follows the existing style (make check)
  • I added the test/contract before the implementation and observed the intended RED failure

@coneilen
coneilen added this pull request to stack #877 October 18, 2026 20:17
Base automatically changed from coneilen-sidebar-row-parity to main October 18, 2026 23:37
@coneilen
coneilen force-pushed the coneilen-sidebar-framing-parity branch from 901dbb2 to ba49b7d Compare October 18, 2026 23:37
Keep Graph and Quick Chats adjacent, put spacing on the folder captions, and share inset row bounds across painting, hit testing, and existing accessibility consumers. Preserve predecessor row styling and selection semantics.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 428a7ff9-63bb-4b9f-a837-ac3719d6bae4
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
Update the eight direct App test/helper consumers to the sidebar inset and caption spacing. Keep hidden/footer negative coverage, add minimum-height collapsed-banner fixtures, and derive sufficient scroll rows from the viewport. No production geometry or workspace behavior changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 428a7ff9-63bb-4b9f-a837-ac3719d6bae4
Signed-off-by: Colin Neilens <coneilen@microsoft.com>
@coneilen
coneilen force-pushed the coneilen-sidebar-framing-parity branch from ca68dd0 to 27ac6bb Compare October 19, 2026 00:46
@coneilen
coneilen marked this pull request as ready for review October 19, 2026 01:29
@coneilen
coneilen merged commit 7f35f4b into main Oct 19, 2026
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant