fix(components): isolate visual series before settling orientation - #427
fix(components): isolate visual series before settling orientation#427wibus-wee wants to merge 2 commits into
Conversation
Orientation was measured on the whole signature group and the series was only split afterwards, along that one axis. A 1-D gap split cannot cut a 2-D arrangement — the median gap inside a grid row is 0 — so two side-by-side lists, which spread wider across the page than either runs down it, were read as one horizontal series. Every grid row then produced a Y deviation and every column break a pitch deviation, scored high by the very page-wide spread that caused them. Lanes are now isolated first, under both axis hypotheses, and orientation is settled per run. Bands cluster centres along the hypothesised axis at half a box; a series takes at most one member per band, so the wrong hypothesis dies as runs of one without anything declaring an axis. Lanes track by nearest cross-axis distance bounded by the group's own band step, anchored on their median rather than their last member. Locality bounds grouping and must not delete membership: a lane of one is one box that left, so a lone residual rejoins the nearest real series and is mined at whatever delta it has. Otherwise the rule would punish the clearest defects hardest. The report's default queue also stopped hiding what the scorer ranks highest. peerSupport counts the deviating level, so an isolated candidate is the shape of a real defect; peer >= 2 hid all of them. Peer-supported cards are kept unconditionally and the best-scoring VISUAL_SINGLETON_REVIEW_BUDGET isolated cards per capture join them. Repeated edge measurements fold into one card that exposes every folded measurement and its witnesses. Attention policy only — the raw JSON and the "All raw deviations" view keep everything, and nothing here becomes a finding, ledger entry or gate verdict. Heights now cluster by anchor distance like every other coordinate, instead of rounding buckets that split nearly identical boxes. On 21 real captures: raw 1122 -> 837, review queue 64 -> 147 across captures that previously showed zero candidates. A regular 3x6 grid and two side-by-side lists go from high-scoring noise to silent, while an 8/16/20/28/200px outlier is still found. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Model: claude-opus-5[1m]
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: 882196a1cd
ℹ️ 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".
| })) | ||
| ) | ||
| .filter((pair) => pair.distance <= step) | ||
| .sort((left, right) => left.distance - right.distance || (left.atom.id < right.atom.id ? -1 : 1)); |
There was a problem hiding this comment.
When two same-kind atoms are equidistant from a lane—for example, when a shifted grid cell lands exactly atop the next column—this tie-break decides which atom claims the established lane. The other atom becomes the singleton residual and is blamed for the resulting pitch deviation, so changing only the primitive IDs can swap the reported candidate from the moved cell to the stationary cell despite identical geometry. Preserve the ambiguity or report both rather than letting identity determine lane membership.
AGENTS.md reference: packages/components/src/lib/geometry-discovery/AGENTS.md:L38-L39
Useful? React with 👍 / 👎.
Auditing the ranked queue on the 21-capture report — rather than the counts and synthetic scenarios alone — showed the top of it was fabrication. Three defects, all the same disease: asserting evidence where there was none. An expectation was not required to be repeated. `expected` is mined from repetition, so a dominant level of one is a single box's own coordinate dressed up as an expectation, and every other member of the series then "deviates" from an arbitrary one of them. A dominant that merely ties the level deviating from it is no better: three boxes here and three there is two positions, and `reduce` named the winner by array order and reported the other half as broken. That was 43% of all output. The dominant must now be repeated and must outnumber what deviates from it. This is the precondition of the comparison existing, not a confidence threshold on a candidate. Bands were clustered by centre distance, which is what a variable-width column moves. Six left-aligned sidebar labels share one left edge and one overlapping X interval, but their centres follow string length, so they read as six separate X positions — and a vertical list was mined as a horizontal series, compared on Y against a workspace header 300px above it. A band is an overlap question: intervals sharing more than half the smaller one, tested against the band's anchor so shifting boxes cannot chain two positions. The residual fold had no distance limit, and that was the worst of the three because it was introduced as a recall FALLBACK. It folded a composer's send icon into the top bar's icon series 849px above and ranked that fabrication first in the entire report, at 3792. Membership needs evidence too, so a lone residual now rejoins only within the same step that bounds lane tracking. Cost, stated rather than hidden: a box drifting more than one step off its series is no longer recovered. The 8/16/20/28px cases still are, and the 200px case is replaced by a test asserting the new boundary and why. Real captures: raw 837 -> 294, dominantSupport==1 362 -> 0, review cards 147 -> 78. This does NOT make the lane a regression detector, and the measurement is recorded in AGENTS.md so it is not rediscovered: 77% of surviving deviations still have a witness in another sectionScope. Locality is a proxy for a question it cannot answer, because the discriminator — same structural relationship — is what visual grouping discards on purpose. The lane is an auxiliary candidate source, not the baseline and regression rail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Model: claude-opus-5[1m]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a3935953e
ℹ️ 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".
| band.atoms.map((atom) => ({ | ||
| lane, | ||
| atom, | ||
| distance: Math.abs(centerOn(atom, cross) - lane.anchor), |
There was a problem hiding this comment.
For a vertical series of left-aligned, variable-width labels, this cross-axis center distance makes the label width determine lane membership. If one long label's center lies more than the row step from the lane anchor, it becomes a residual that the bound at line 378 refuses to rejoin; consequently, even an obvious left-edge offset on that label produces no deviation. For example, six 40px-pitch text rows with widths [20, 30, 40, 300, 50, 60] and only the 300px row shifted from x=100 to x=108 were reported by the parent implementation but are now missed entirely. Track a width-independent edge/overlap relationship for the cross axis so content-sized width cannot partition the comparison.
AGENTS.md reference: packages/components/src/lib/geometry-discovery/AGENTS.md:L25-L29
Useful? React with 👍 / 👎.
1 participant
Related issue
Refs #408
This stacks directly on #408 (base
feat/capture-mobile-viewports) and must not close it.No separate tracking Issue exists; this is a same-repository branch, which
.github/AGENTS.mdclassifies as
internaland does not require an Issue for intake.Problem / pressure
#408 landed the DOM-blind visual repetition lane. Reviewing its real output on the 21-capture
report — the ranked queue itself, not just the counts — showed the top of that queue was
fabrication, from four separate causes.
Orientation was decided page-wide.
seriesAxisran on the whole signature group and thesplit happened afterwards along that one axis. A 1-D gap split cannot cut a 2-D arrangement
(the median gap inside a grid row is
0), so a grid was read as one horizontal series: everyrow produced a Y deviation and every column break a pitch deviation, each scored high by the
very page-wide spread that caused it.
An expectation was never required to be repeated.
expectedis mined from repetition, soa dominant level of one is a single box's own coordinate dressed up as an expectation, and
every other member then "deviates" from an arbitrary one of them. A dominant that merely ties
the deviating level is no better: three boxes here and three there is two positions, and
reducepicked the winner by array order and reported the other half as broken. 43% of alloutput.
Bands were clustered by centre distance, which is exactly what a variable-width column
moves. Six left-aligned sidebar labels share one left edge and one overlapping X interval, but
their centres follow string length — read as centres they became six separate X positions, and
a vertical list was mined as a horizontal series against a workspace header 300px above it.
The residual fold had no distance limit, and this was the worst one because it was
introduced as a recall fallback. It folded a composer's send icon into the top bar's icon
series 849px above it and ranked that fabrication first in the entire report, at 3792.
The default review queue filtered on
peerSupport >= 2, which hid precisely what thescorer ranks highest: one box alone off its series edge.
Summary
most one member per band, so the wrong hypothesis dies as runs of one without anything
declaring an axis. A run both hypotheses produced is settled by that run's own spread.
the level deviating from it. This is the precondition of the comparison, not a confidence
threshold on a candidate.
band's anchor) instead of centre distance.
evidence too.
peerSupport >= 2queue with peer-supported cards (unconditional) plus thebest-scoring
VISUAL_SINGLETON_REVIEW_BUDGET(5) isolated cards per capture, and foldan atom's repeated edge measurements into one card that exposes every folded measurement
and its witnesses.
Explicitly not done: no CV, no classifier, no cross-capture aggregation, no promotion into
GeometryFinding/ ledger / contract / gate, and no coherent-measure-only filtering (it wouldsilence variable-width text by deleting real width-only drift).
Before / after
dominantSupport == 1— expectation from a sample of one[100,100,100,108,100,100]startagrees)end+centerThe review-card count rises because 8 of 21 captures previously had zero candidates: their
only signal was isolated. It stays bounded because the budget is per capture.
The measured limit, which this PR does not fix
After all of the above, 77% of surviving deviations still have a witness in a different
sectionScopethan the candidate — top bar icon against composer icon, sidebar row againstpanel header. 14 of the top 15 review cards are cross-section.
This is not a remaining bug to chase. Locality is a proxy, and the discriminator a reviewer
actually applies — same structural relationship — is precisely what visual grouping discards
on purpose. Some cross-section comparisons are the intended catch (two sidebar lists rendering
one row at different pitches is exactly the "two code paths, one visual thing" case the lane
exists for); most are noise, and the card cannot say which.
So this lane is an auxiliary candidate source. It is not a baseline/regression instrument
and cannot be made into one by algorithmic means, because it holds none of the state that
question needs. That measurement is recorded in
AGENTS.mdso it is not rediscovered. Whetherthese candidates should appear in the main geometry report at all is a placement decision that
sits above this PR — the authored baseline rail (
findings.json,geometry-ledger.json,contracts,
finding-diff,dimensionSensitivity) is untouched here and cannot be polluted bythis lane:
discoverVisualRepetitionoutput flows only intoreportData.visualRepetition.Test plan
corepack pnpm --filter @lody/components exec vitest run tests/geometry-discovery-visual-repetition.test.ts tests/geometry-discovery-report.test.ts tests/geometry-discovery-visual-capture.test.ts— 29 passed. Covers isolated anomaly, singleton budget bounding, regular column, side-by-side lists, regular grid, grid + local anomaly, outlier recall at 8/16/20/28/60px, the asserted boundary where recall stops, width-only drift, tag normalization, translation invariance, sub-pixel jitter, deterministic ranking.corepack pnpm --filter @lody/components geometry:report— full 21-capture run passes, 78/80 screenshots,visual-repetition.jsonand overlays intact. The post-fix figures above were then re-mined from that artifact's atoms.corepack pnpm --filter @lody/components exec tsc --noEmit— clean.oxlint --type-aware— 0 errors.use-session-actions,session-mention-drop-layer,mobile-chat-list-opened-byfail withact is not a functionin this checkout. Verified identical atHEADwith these changes stashed.Context handoff
Instructions for reviewing agents
scoreLevels(dominant must be repeated and must outnumber),buildBandsoverlap semantics, and the step bound on the residual fold. Then confirm the lane still cannot reach findings/ledger/gate.Authoring context
end/centerdeviations from label length are still reported, because visual geometry alone cannot separate them from a drifted component. Coherent-measure-only filtering would have produced prettier numbers by deleting width-only drift, so it was rejected. The cross-section limit is measured and documented, not fixed.