fix(components): repair sidebar trailing alignment, split geometry constraint system, retire first marker - #327
Conversation
Introduce `--spacing-sidebar-trailing` (9px) as the one token for a bordered row's trailing inset (8px padding + 1px transparent border) and apply it where a header or accordion trigger has no border of its own to contribute that last pixel: the Sidebar section header, session group header trailing action, local project row, and the changes sidebar's category header stats. Anchor the changes-sidebar category stats to the trailing edge with the same wrapper the file rows use, so they no longer ride the category label and slide with its width. Found by the geometry constraint system's alignment discovery. Model: claude-sonnet-5
…ble pipeline Factor `geometry-constraint-system.ts` (pipeline, ledger, contracts, tokens, metrics, classification) and `geometry-text-cap-band.ts` (the one optical text measurement) out of `chat-workspace-geometry.ts`, so capture and the `?geometry=1` overlay share one cap-height measurement and the tabular-nums bug in the overlay is gone with it. Ledger and gate: - `geometry-ledger.json` / `geometry-contracts.json` are the checked-in review state: findings default to `accepted-debt`, and a contract compiles only `promoted` entries. `scripts/triage-geometry-findings.mjs` (root `pnpm geometry:triage <dir>`) merges a fresh findings.json into the ledger, re-keying resolved reviews across structural changes instead of reporting them as new. - Root `pnpm test:geometry` runs the Playwright gate in CI; the workflow now installs Chromium for it. Docs: add `tests/e2e/AGENTS.md` and `tests/e2e/support/AGENTS.md` (with CLAUDE.md symlinks) recording the X/Y rail discovery rules, finding identity and re-keying, the ledger/contract/gate algebra, and what counts as ink vs. a layout box. Update the package and lib AGENTS.md to point at them. Model: claude-sonnet-5
Discovery has fully replaced this rule: marker-removal-readiness showed 18/18 members matched with zero coordinate/offset deltas in its one capture, and y-axis-parity confirmed the same before this change. Drop the rule from `CHAT_WORKSPACE_SEMANTIC_ALIGNMENTS` and the `data-geometry-align-*` markers business code carried only for it (local project/session titles, session time and added/removed line counts, the leading status indicator). Retire the one-off `compareMarkerAlignmentsToBlockRails` wiring in the report script that existed to prove this specific rule ready — the function stays for the next removal candidate — and update tests/e2e/support/AGENTS.md to describe the general readiness process instead of naming a marker that no longer exists. Verified: `tsgo --noEmit`, unit tests (101 passed), and the full Playwright gate (9 passed) including "vertical row alignment is discovered without geometry marker attributes". Model: claude-sonnet-5
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5036a11174
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const normalized = scope.candidates | ||
| .map((candidate) => ({ | ||
| locator: candidate.locator, | ||
| label: candidate.label, |
There was a problem hiding this comment.
When separate captures have identical relative geometry but different DOM ordinals or element-derived sections, this projection produces the same cache key while omitting primitiveId, boxModelNodeRef, sectionScope, and naming metadata. The cache hit then reuses the first capture's rail members, and materializeGeometryObservationScope only translates their coordinates; consequently createGeometryFindings can look up stale node references in the current capture, lose CSS explanations, or assign the finding to the wrong section. Preserve ordinal-independent reuse by rebinding cached rails to the current candidates, while including identity-relevant metadata such as the section in the cache comparison.
AGENTS.md reference: packages/components/tests/e2e/AGENTS.md:L57-L70
Useful? React with 👍 / 👎.
| ...(surface.captureId === 'workspace:wide-expanded' | ||
| ? { | ||
| semanticAlignments: semanticAlignments.map((entry) => { |
There was a problem hiding this comment.
Because semantic marker observations are attached only to workspace:wide-expanded, assessGeometryMarkerRemoval treats every dark, localized, and alternate-state capture as though the rule never appeared there, so none of those captures can veto a ready result. This commit then removes the shared sidebar-row markers based on that incomplete artifact, allowing discovery gaps in those variants to become permanently invisible; collect the semantic observations alongside each capture where the marker renders before declaring the rule removable.
AGENTS.md reference: packages/components/tests/e2e/support/AGENTS.md:L54-L59
Useful? React with 👍 / 👎.
1 participant
Summary
Stacked on #232 (
feat/web-grid-system, still draft).Post (continued)
The alignment rail story in the first post works for one axis, one moment. Running it across every real surface exposed the harder problem: the same rail got rediscovered dozens of times a run, with no way to tell today's report from yesterday's. 74 cards turned out to represent 25 distinct elements. So the system got a spine — five files, each reading only the one before it:
A finding's key is structural now — role, row family, same-role index, the anchor — deliberately coordinate-free and locale-free. A
geometry-ledger.jsongets checked in: a human moves a finding toaccepted-debt,ignored, orpromotedexactly once, and every run after that shows a diff against the ledger, not a fresh dump.That still leaves the question under every card: why is it off by 4px? So each finding now carries an explanation — walk both members up to their common ancestor, sum the padding/border/margin/gap terms CSS actually declares:
Explained offset equal to the measured offset, residual near zero — that's a
css-defect, a named property and a one-line fix. A title whose box model matches exactly, where the whole 1.35px turns out to be the glyph's own optical inset, is the opposite verdict: not a defect, just ink sitting inside a box.The original motivation was vertical, not horizontal — an icon sitting visibly higher than its label. Text has no center of its own the way a box does, so Y discovery needed its own primitive: a cap-height band measured off a fixed reference glyph, never off the row's own string:
With that anchor, discovery could finally ask the question the project started from — is this icon's center where the label's ink actually sits, structurally, across every capture — without a developer taping a
data-geometry-align-yattribute onto it.Which sets up the actual close of the loop. Every hand-placed marker is business-code weight the system was supposed to make unnecessary, so it learned to grade its own homework before anything gets deleted:
sidebar.row.visual-centeris the first rule to clear that bar and the first one actually deleted — rule declaration and all six markers gone, gate still green because a test literally named "vertical row alignment is discovered without geometry marker attributes" watches it hold with no marker in the DOM at all. One rule out of three declared today. First, not last.