From 0ace0e017cfe5b44772a3087776e3055f79ee2d6 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sun, 20 Sep 2026 02:09:53 -0400 Subject: [PATCH] Record the sixth sweep, and the base every sweep should have used PR #19's base is upstream/main at ca2b4b2, not the local `main`, which tracks the fork's divergent line and sits four merged pull requests behind it. Diffing against `main` shows #10, #12, #16 and #17 as this branch's work; that is how the parley text migration's undo path kept surfacing in sweeps. Git cannot record a pull request's base and `main` cannot be renamed, so the handoff carries the note and the checkout carries `git iris-base`/`git iris-diff`. Co-Authored-By: Claude Opus 5 --- docs/HANDOFF.md | 34 +++++++++++++++ docs/LAYOUT_LOG.md | 101 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index ee92fe1..e33010d 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -39,6 +39,40 @@ Two checkouts share one Git storage: `/home/bob/repos/iris` is the active `layout/one-ask` worktree, and `ai-app-2/iris` stays on `main` at the app's `32f6ad8` pin until the integration below is ready. +### The base is `upstream/main`, not `main` + +**Diff this branch against `upstream/main`. The local `main` is the wrong +base and gives a plausible-looking wrong answer.** PR #19 is +`iris-ai/iris:layout/one-ask` into `iris/iris:main`, which is the `upstream` +remote, at `ca2b4b2`. Local `main` tracks `origin/main` -- the *fork's* line, +which carries the app's own commits, is not an ancestor of upstream's main, +and sits four merged pull requests behind it (#10 parley text, #12 pointer +routing, #16 draw/size merge, #17 headless rig). + + git diff main...layout/one-ask # WRONG -- base 7b54aaf + git diff upstream/main...layout/one-ask # right -- base ca2b4b2 + +`git iris-base` and `git iris-diff` are configured in this checkout and use +the right one. The authority is gitea, when it matters: + + curl -s -H "Authorization: token $(cat ~/.config/gitea/token)" \ + https://git.arirex.me/api/v1/repos/iris/iris/pulls/19 \ + | python3 -c 'import json,sys; print(json.load(sys.stdin)["base"]["sha"])' + +Nothing here can be fixed by renaming: `main` is the app submodule +worktree's checked-out branch at its `32f6ad8` pin, and the two worktrees +share one ref store. Git has no way to record a pull request's base, so the +note is the mechanism. + +This has already cost real work. The sixth sweep reviewed against local +`main` for half a session (Bryan caught it, 2026-09-20), and reported the +parley migration's undo path as this branch's. The fourth and fifth sweeps +deleted `Painter::text_data`, `ActiveData::size_deps` and `SizeRule::apply` +partly on the same false reading; the deletions stand on their own merits -- +`text_data` had no caller at `ca2b4b2` either, and `size_deps` was read there +and orphaned by this branch's rewrite -- but "arrived on this branch" was not +the reason for all of them. + ### How to check a round **Always**, because they cost nothing: format, workspace clippy under diff --git a/docs/LAYOUT_LOG.md b/docs/LAYOUT_LOG.md index 7accb37..97dff62 100644 --- a/docs/LAYOUT_LOG.md +++ b/docs/LAYOUT_LOG.md @@ -6,6 +6,99 @@ nothing here is rediscovered. Each entry says who found it and when. it (settled design, the measurement method) belongs in `docs/LAYOUT.md`, and the current plan is in `docs/HANDOFF.md`. +## Sixth sweep: the shader boundary and the position widgets (2026-09-20) + +Over what the five earlier rounds did not name -- the WGSL prelude and how +it is assembled, the position widgets, `orientation/`, and the sensor walk. +**Scoped against `upstream/main` at `ca2b4b2`, which is PR #19's real base**; +the first half of this sweep used the local `main` and had to be redone, for +which see "The base is `upstream/main`" in `docs/HANDOFF.md`. Two findings. +The cold dump is byte-identical to `1096c31` and all three seed scans pass +(400 at depth 5 in 69.07s, 1,000 at depth 6 in 169.29s, 2,000 at depth 4 in +300.75s). + +**A number both sides count in, written twice.** `module_source` already +builds each shader's preamble from `iris_core`'s own constants, with a +comment saying why: "a grid the two disagree about puts every coordinate +somewhere else". The move-chain work then added, to the very file it +prepends, a second copy of two of its own numbers -- `const MOVE_NONE` and +`const CHAIN_LIMIT`, under "Keep in step with `iris_core::CHAIN_LIMIT`" -- +asking a reader by hand for what the mechanism beside it exists to do, and +duplicating the reasoning `CHAIN_LIMIT`'s Rust declaration already carries. +Both are injected now and the shader declares neither. +`every_shader_validates` composes the real preamble so it covers the change; +what it could never have caught is the two numbers drifting apart, which is +now unrepresentable. + +`MASK_NONE` went in beside them, replacing a bare `4294967295u` in `masked` +-- where the CPU deliberately keeps `MaskIdx` and `MoveIdx` as separate +types so the two cannot be swapped. **That literal pre-dates this PR**; it +is a one-line drive-by in a block the PR was already rewriting, taken +because leaving it means a named sentinel for one index and a magic number +for its sibling four lines apart. Drop it if the scope matters more. + +**A scroll positioning content it does not position.** Two halves, one +mistake, both new in this PR -- the base's `Scroll` has none of this +machinery. The author believed `Scroll` places its own content. + +`content_len` is `answer_px.max(container_len)`, so it is never less than +the box. Two lines on, `slack` was `(container_len - content_len).max(ZERO)` +and `anchor` was `slack * align.rel()` -- provably always zero, whatever the +alignment, with `moved` then testing `anchor != ZERO` for nothing. +Instrumented at `1096c31`, a centred scroll over 50 px of content in a +200 px box prints `slack=0 align=AxisAlign(0.5) anchor=0` and centres the +content anyway: the framework does it, by placing the inner's answer in the +whole box, which comes out as `rel 0.5 - px 25` and resolves at any length. +The comment credited the arithmetic for behaviour it could not produce. + +The same belief cost a redraw. The contract for content that fits was +guarded by `align == AxisAlign::NEG`, on the reasoning that at any other +alignment "it moves with every length the box takes and the drawing holds +for that length alone". It does not move -- the framework's placement is a +fraction of the box -- and the default alignment is the middle, so the +common case was the guarded one. `Painter::px_len` holds a drawing to the +length it read unless the widget says otherwise, so with no `holds` stated +the scroll redrew on every box change. Measured with `distinct_widgets`: 1 +at `CENTER`, 0 at `TOP_LEFT`. Dropping the alignment test gives 0 at all +three, which is what `a_fitting_scroll_holds_for_every_box_its_content_fits_in` +asserts; it fails at `1096c31` on the `CENTER` case. `align` had no other +reader, so the widget no longer asks its own alignment at all -- which is +the tell that both halves were one mistake. + +**`UiSpan::translated` and `UiRegion::translated`** arrived on this branch +and are reachable only from each other, which is why a plain "is this name +used anywhere else" scan does not see them: neither looks unused on its own. +The region one carried a performance argument for a function nobody calls. +Both deleted. + +### Kept although they pre-date this PR + +`GlyphAtlas::glyph_count`, `TextBuffer::new_empty` and the let-chain in +`TextEdit::apply_event` that #10 had expanded into a nested `if`. All three +were found under the wrong base and are outside #19's diff; Bryan said to +keep them anyway (2026-09-20), since the two are dead either way and the +third is a straight restoration. + +### Tripped a rule and left as it stands + +- `diag::untrace_widget` is new here with no caller, which is the test the + `translated` pair was deleted by. Left: it is the "removed" half of what + `trace_widget`'s own doc promises ("until explicitly removed or cleared"), + and `clear_traced_widgets` is the "cleared" half and does have one. A rig + drives this API from outside. +- `UiSpan::flip` swaps `start.rel` with `end.rel` and `start.px` with + `end.px`, where `Len` has exactly those two fields and one swap of the + structs would do. Pre-dates this PR. +- `UiVec2` translates under three names -- `shift`, `offset`, and + `UiRegion::offset` -- beside `Len::offset`, which adds pixels to one end + and is a different operation. Pre-existing, and one vocabulary is Bryan's + call rather than a sweep's. +- The nine `#[allow(unused_variables)]` are all on trait methods with empty + default bodies, where the parameter names are the signature's + documentation and underscoring them would hide it from implementors. +- `GpuPages::update` grows before it drains and both go through the one + queue, so the copy is ordered before the writes. Correct as it stands. + ## Fifth sweep: the retained path, the renderer and the diagnostics (2026-09-20) Over the parts the four earlier rounds did not read -- the renderer, the @@ -79,8 +172,12 @@ throughout, `LayoutLen::{is_px, is_only_leftover, declared, fills}` and same words share an answer" means, which is a design question. - `TextEdit`'s undo history pushes a whole copy of the text per changed keystroke and is never bounded, and `apply_event` clones the text on every - event including the arrow keys. Both are on `main` verbatim and unchanged - by this branch, so they belong to a change of their own. + event including the arrow keys. **`apply_event` is not in this PR's diff at + all**: it was last touched by #10 and #16, both already on `upstream/main`. + It keeps resurfacing in sweeps because they diffed against the local `main` + -- see "The base is `upstream/main`" in `docs/HANDOFF.md`. Bryan wants the + unbounded push and the clone dealt with as a change of their own + (2026-09-20). - `ActivationState::update` writes four arms where the `Start`/`On` and `End`/`Off` pairs are identical, and `is_off` is `!is_on`. Also verbatim from `main`.