Work plan: S8 phrase-boundary overlays (× S4) - #39
Conversation
Analysis surfaces the focus track's phrase-boundary start ticks under the PPQN-scaled default config (the closure.rs referee precedent), and the scene places a BoundaryMark column for each visible boundary tick. References a field and a role that do not exist yet, so the crate fails to compile until the green step. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
Analysis surfaces the focus track's phrase-boundary start ticks under
the PPQN-scaled default config (the closure.rs referee precedent: snap
grid 1/16, minimum gap two quarters), and the scene places a
BoundaryMark column ('┊') for each visible boundary — over blank cells
or gridlines, after the section marks so a section keeps precedence on
a shared column. The TUI maps the role to a dim amber; goldens
re-blessed with the demo marker.
https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes First increment after #38: S8 phrase-boundary overlays — Generated by Claude Code |
![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: efc2551f45
ℹ️ 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".
| // after the section marks so a section keeps precedence when both land | ||
| // on one column. | ||
| for &tick in &analysis.boundaries { | ||
| if tick <= vp.scroll_tick { |
There was a problem hiding this comment.
When the user scrolls so a phrase boundary is exactly at vp.scroll_tick (for example, the fixture boundary at tick 480 after one right-scroll at 60 ticks/col), this <= check drops it even though visible_col would map it to the leftmost plot column. That makes otherwise visible boundary overlays disappear at the viewport edge; only ticks before the scroll origin should be skipped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in aebbd5a (red → green): the fixture boundary at tick 480 with scroll_tick = 480 reproduced the dropped marker. The guard was copied from the section-mark loop, where skipping the origin divider is deliberate (the band already names the section at the left edge); for boundaries the loop now has no guard at all — visible_col already filters ticks before the scroll origin, and a boundary at the edge is information the curator scrolled to see. Decision recorded in docs/decisions.log.md.
Generated by Claude Code
…renders The boundary loop copied the section-mark guard (tick <= scroll_tick), but a boundary scrolled exactly to the viewport's left edge maps to the leftmost plot column and is visible; only ticks before the scroll origin should be skipped — which visible_col already does. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
…, PR #39) The boundary loop dropped a tick equal to scroll_tick although it maps to the leftmost plot column; the guard was copied from the section-mark loop, where skipping the origin divider is deliberate. visible_col already filters ticks before the scroll origin, so the loop needs no guard at all. https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
|
@codex plz review changes Pushed the fix for the left-edge finding: the boundary loop drops its scroll-origin guard entirely ( Generated by Claude Code |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
2 participants
One roadmap increment as a red → green → docs TDD cycle (the work-plan continuation after #38).
S8 — phrase-boundary overlays (× S4)
Analysis.boundariessurfaces the focus track's S4 phrase-boundary start ticks under the PPQN-scaled default config (theclosure.rsreferee precedent: snap grid 1/16, minimum gap two quarters).BoundaryMarkcolumn (┊) for each visible boundary — over blank cells or gridlines, after the section marks so a section keeps precedence on a shared column (ADR-0016 layer 3: one home for the layout math).Decision recorded in
docs/decisions.log.md; S8 stage doc updated.https://claude.ai/code/session_01NC6kB92Hn6Gcf9cMHjLt4b
Generated by Claude Code