Repository navigation
fix(client): turn the forward wire in the corridor the row leaves - #2172
Merged
Merged
Conversation
…inkFlowLab#2164) A forward edge that leaves its row for one below measures its corridor from the row's lowest measured bottom. At the factory 108px row pitch a 120px card beside a 48px source reaches past the next row's top, so that bottom sat below the target's top: the gap clamp went to 0, the corridor fell inside the cards below and the run folded back over itself. Keep the source-only height only when the row's bottom leaves no corridor at all (rowBottom >= to.top), which is that tall-card shape. Every shorter neighbour keeps the row-clear turn, so a 91px card beside the source -- the tallest the shipped stylesheet draws -- still turns below it instead of heading off at the source's own height through its body. The corridor height is also clamped to the target's top, so a forward run cannot fold back on itself on any input.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2164
Written-by: Dev-2
Owner: Dev-2
Reviewer: Birfy
Claim-Id: a6dd7dd7e74b4328a9bec47d8a8ce344
Branch: dev/2-forward-wire-factory-pitch
Base: ff16097 (main; this head's single parent is 09f8fe7, which was main at push time)
Tip SHA: 3e00ba4
Candidate SHA: b11ae8d61f374132f1a22490f1f2a4a382378c51 (single parent 41ff819)
What was wrong
#2161(squashed as96e24ef307e034f3be2706ac60a6631cc8db0e24) made a row-crossing forward wire measure its corridor from the row's lowest measured card bottom. At the factory pitch that bottom can already sit below the next row's top:GAP_Y = 108(plugins/workflow/src/graph.js:10) and a 120px card standing beside a 48px source puts the row's bottom at 120, four pixels past the target's top edge at 108. The gap clamp then went to 0, the corridor landed on that row bottom, and the run was drawn through the cards of the row below — on one input folding back over itself:The same measured row bottom also made
#2161's turn depend on a threshold rather than on the corridor: a 92px card beside the source (30–38 latin characters in the shipped stylesheet) leaves 16px of room, and any variant that falls back to the source's own height runs through that card's body and out the other side.Red first
Three tests were added to
core/test/workflows-page.test.jsand run on the clean, unchanged tree (git status --porcelainempty).Against main
41ff8199: 42 tests, 40 pass, 2 failThe first is the reported shape (the run at the row bottom, inside the cards below, folding back); the second is the reachable 112px CJK card whose bottom reaches past the next row's top.
Against the earlier candidate
a0418044(belowRow = sameRow || room >= 2 * WIRE_GAP, 28px threshold) with the same tests: 42 tests, 39 pass, 3 fail — the same two plusThat third failure is why the threshold version was dropped: at 92px (a reachable latin height) it routes at the row top and through the neighbour's body.
At this tip: 42 tests, 42 pass, 0 fail.
What changed
core/src/client/workflows.js(the forward branch ofwireRoutes). The corridor is still measured from the row's lowest bottom, but the turn is now decided in three cases instead of by a threshold:room > 0— the row leaves a corridor, so the turn halves it (capped atWIRE_GAP), exactly the geometry main draws. This is every height a card can reach today.room === 0— the neighbour's bottom sits exactly on the next row's top. The turn stays on that edge, which is byte-for-byte main's route.room < 0— the neighbour reaches past the next row's top. The turn is clamped one pixel above it (y2 - 1), so the run crosses that neighbour and nothing is drawn into the row below.core/test/workflows-page.test.jsgained the three tests above: the factory pitch with 120/140/160 guard heights, the corridor under a 92px and a 96px neighbour, and a 40-character CJK name at 112px.changelog.d/2164-forward-wire-factory-pitch.fixed.mdanddocs/subsystems/core-client.mdrecord the change, andcore/client.jsandpatches/ds-bot/*were regenerated withnode scripts/ds-bot-patches.mjs generateafter(cd core && npm run build).Reachable heights
The shipped stylesheet, measured with its own CSS and markup in headless chromium (card width is a fixed 168px,
transform: scale()does not changeoffsetHeight, hover/selected/status rows do not change the height): latin 1–13 characters 47px, 14–20 62px, 21–29 77px, 30–38 92px, 39–40 107px (a 40-character pure-latin name does not wrap and stays 46–47px, overflowing horizontally); CJK (a 112px-wide name box, 8 characters a line) 1–8 characters 48px, 9–16 64px, 17–24 80px, 25–32 96px, 33–40 112px. The factory cards themselves sit at 47px; across the measured surface the range is 46–112px.So the reachable ceiling is 112px — a 40-character Chinese name plus its role line, with two independent readings of 112 (Test) and 111 (Product), i.e. ±1px of font metrics. 108–112 is treated as reachable; 120/140/160 are the 113-and-up guard, not a measured shape. (
91is gone: it was the previous round's latin-only ceiling.)Where it starts
Measured against main
41ff8199on the factory fixture, one pixel at a time:bdM 0 48 L 0 107 L 94 107 L 94 107.5 L 188 107.5 L 188 108M 0 48 L 0 108 L 94 108 L 94 108 L 188 108 L 188 108M 0 48 L 0 107 L 82 107 L 82 108 L 188 108 L 188 108b ≤ 106(every height up to and including the 25–32 CJK band at 96px): the samedmain draws, 61 consecutive heights, no drift.b = 107(room = 1) is the first difference, 0.5px:yAis clamped toy2 − 1(107) where main turns at 107.5. Neither run enters the neighbour, both cross 0 cards, and the 0.5px sits outside the target's border. That height is the 39–40 character latin card, so it counts as reachable.b = 108(room = 0) keeps main's turn exactly —yA = to.top, the samedmain draws, 0 crossings on both sides. One pixel higher and the wire would cross a card main does not, which is why this one case is not clamped.b = 109–112is where this fix is better than main, not merely equal: main runs atyA = 109|110|111|112, one to four pixels inside the next row, and crosses two cards (the neighbour above it and the card below); this tip clamps toy2 − 1 = 107, crossings 1 vs main's 2, and nothing is drawn into the next row. Atb ≥ 113the count matches0bfaf525(1 = 1) — those heights have no measurement and exist only as the scan's upper-bound guard.Regression guard
#2161was a T1 change that reached main with the factory row pitch broken, and the same class of change will come again: any edit to how a row's bottom or a card's prototype height is measured moves this geometry. The three tests here pin the factory pitch (108px rows, 188px columns), a taller neighbour that still leaves a corridor (92px and 96px, both reachable), the exact boundary where the corridor closes (108px, byte-for-byte main), and the tallest measured card (a 40-character CJK name at 112px), with 120/140/160 kept only as an upper-bound scan.Verification
Ran on the clean, committed candidate
b11ae8d6(git status --porcelainempty), whose eight blobs are the ones in this head (see below):core: 2639 tests, 2639 pass, 0 fail;plugins/social555 (554 pass, 1 skip),plugins/tree127 (126 pass, 1 skip),plugins/timeline140 (139 pass, 1 skip),plugins/workflow347,.github/scripts598,interop37,live-e2e24 — 0 failuresarch-docs: OK (8 changed file(s));ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70;ok 10 patches apply cleanly in order;ok core/ equals ds-bot/ + patches/ds-bot/;ok patches/ds-bot/ is what generate writes (generate is a no-op)—build-client --checkandds-bot-patches checkare steps insideverifysha256 core/client.js4ef0b58ae51e8b5dfce243e906a582adabfcfe3309f3fd52c3515b62ac36b88e,sha256 patches/ds-bot/0008-client-bundle.patch2116890ff54e102ff799312e8e000f090261dfa7fe763b6767e604ae63d8678a— both re-hashed from the blobs in this head, identical to the candidate'sThe push rebuilt the commit on top of the
mainof that moment (09f8fe7e): same eight blobs (all eight blob shas matchb11ae8d6), so the diff againstmainis exactly these eight files with the same +251/−10; the candidate's single parent is41ff8199, this head's single parent is09f8fe7e.mainadvanced once after the push (ff160976,#2166/#2168, which touch onlycore/src/host/journal.jsand its tests/docs), so this head was left as pushed, not rebased: it is still one commit, mergeable, and the diff is unchanged. Not run: nothing beyondnpm run verify— no live instance, no browser probe of the served client beyond the test suite.Checkpoint
{ "pr": 2172, "claim": "Dev-2", "issue": 2164, "summary": "A row-crossing forward wire no longer takes the row's lowest bottom as its corridor when a taller neighbour reaches past the next row's top. The turn is now decided in three cases: a real corridor is halved as before, a zero corridor keeps main's exact turn, and a negative one is clamped one pixel above the next row so nothing is drawn into the cards below. Tests pin the factory pitch, the 92px and 96px corridors, the 108px boundary and the 112px CJK card against the exact `d` main draws.", "files": [ "core/src/client/workflows.js", "core/test/workflows-page.test.js", "core/client.js", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/src/client/workflows.js", "patches/ds-bot/files/test/workflows-page.test.js", "changelog.d/2164-forward-wire-factory-pitch.fixed.md", "docs/subsystems/core-client.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=41ff8199 npm run verify (twice-built, clean tree)", "result": "green", "at": "3e00ba405510a781de662b45be788ebd4464ead0" }, "next": "none — ready for review" }