Repository navigation
C2c2: Arrangement end marker + render eligibility QML (#1080) - #1166
Conversation
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
sample-brain/src/arrangement_domain.py
Line 125 in c5b3e0d
Reject unsnapped markers during state construction
When an ArrangementState is directly constructed or produced with dataclasses.replace, this now accepts an end marker such as 3 even though set_end_marker() and from_payload() reject it. Such a state can report render_eligible=True, and WorkbenchSession.set_arrangement_state() will accept and serialize it, after which the next arrangement_state read fails while parsing the unsnapped payload. Keep the bar-snap invariant in __post_init__ (or enforce it in validate and eligibility) so every public construction path remains consistent.
ℹ️ 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".
| x: window.arrangement.endMarkerNormalizedPosition * parent.width | ||
| width: 2 | ||
| visible: window.arrangement.endMarkerVisible |
There was a problem hiding this comment.
When the marker is within the newly allowed two-block lead margin to the left of the viewport, its normalized position is negative while endMarkerVisible is true. Multiplying that value directly by the stage width places the marker outside the stage—for example, marker 96 with viewport 200..560 gets x ≈ -0.289 * width—so the marker the projection explicitly promises to show is either invisible or bleeds outside its container. Clamp the rendered position to the left edge, or stop marking negative normalized positions visible.
Useful? React with 👍 / 👎.
| forbidden_patterns = [ | ||
| r'\btimeline\b', r'\bTIMELINE\b', | ||
| r'\bendMarker\b', r'\bend_marker\b', r'\bEndMarker\b', | ||
| r'\brenderEligibility\b', r'\brender_eligibility\b', | ||
| r'\bLive\b', r'\bLIVE\b', r'\bMIDI\b', r'\bmidi\b', |
There was a problem hiding this comment.
This file declares itself a frozen C2b contract, but this change silently removes the end-marker and render-eligibility patterns from its scope-creep check while the test's own description still says those concepts are forbidden. That weakens a historical contract to accommodate the implementation instead of preserving it and proving the new C2c2 behavior in the new test suite; retain the frozen assertions or explicitly establish the contract's supersession without editing the frozen test.
AGENTS.md reference: tests/AGENTS.md:L18-L24
Useful? React with 👍 / 👎.
1 participant
Slice C2c2 — End Marker + Render Eligibility (QML arrangement shell)
Refs #1080
Test-first C2c2 slice: Python-owned arrangement end marker projected on the C2c1 timeline, mutated via domain authority (set/clear/nudge with quarter-note snap), with render eligibility gating.
Commits
Scope
equest_end_marker_set/clear/nudge\
Validation
Pre-existing failure (not this slice)
\ ests/test_workbench_qml_transport_header.py::test_runtime_qml_header_projects_live_transport_and_commands\ (\�ctive_screen 'screen1' != 'screen2') reproduces identically on main (\�327d690) — verified via a detached worktree before attributing it. This slice touches no channel-rack/active-screen code path.