feat(engine): scrollback-commit primitive for inline mode#112
Merged
Conversation
Adds engine_commit_scrollback(bytes, len, rows) (engine ABI 1.4.0, library 1.5.0): stage drawlist-rendered rows for emission into terminal scrollback above the inline live region — the Ink <Static> / Bubble Tea Println primitive that agent-style CLIs need. Design: - commit content is a normal versioned drawlist executed at submit time into an isolated framebuffer (viewport cols x rows); resources act on a transient snapshot; no-partial-effects on failure - emission (zr_diff_render_commit, pure): relative re-anchor to region top, per-row paint with baseline-SGR tail erase (EL) + LF claims so earlier lines scroll into history, then a fresh-line claim re-anchors the region; SCREEN_VALID cleared so the following live diff rebaselines below - present integration: staged blocks emit FIFO before the live-region diff inside the same single flush and synchronized-update wrap; staged blocks survive failed presents and release only after a successful flush - bounded staging: ZR_COMMIT_ROWS_MAX per call, ZR_COMMIT_PENDING_ROWS_MAX total, 32 pending blocks Tests: engine-level byte-exact present stream, ALT-mode rejection, rows bounds, FIFO + release, pending caps, retry-after-failed-present; golden fixtures for the pure emitter; VT model gained EL (CSI K). Example: inline_status.c commits checkpoint lines at progress quarters (verified in tmux: lines persist in scrollback during and after the run). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR implements inline-mode scrollback commits: new APIs stage drawlist-rendered rows into engine-held framebuffers, present emits them before the live-region diff in a single flush, staged commits survive failed presents until released, version pins and limits are updated, and tests/docs/examples are added. ChangesInline Scrollback Commit Feature
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/zr_vt_model.c (1)
807-824: ⚡ Quick winConsider extracting cursor-clamping + single-line-fill to a helper.
Lines 811–819 duplicate the cursor-clamping and rectangle-fill logic from
zr_vt_model_erase_below(lines 108–116). Extracting this pattern to a small helper would eliminate the duplication and align with the guideline to prefer helper functions for repeated calculations.♻️ Refactor to extract shared logic
Add a helper function before
zr_vt_model_erase_below:/* Fill from cursor to end of current line using current style. */ static zr_result_t zr_vt_model_erase_cursor_to_eol(zr_vt_model_t* m) { if (!m || m->cols == 0u || m->rows == 0u) { return ZR_ERR_INVALID_ARGUMENT; } const uint32_t y = (m->ts.cursor_y < m->rows) ? m->ts.cursor_y : (m->rows - 1u); const uint32_t x = (m->ts.cursor_x < m->cols) ? m->ts.cursor_x : (m->cols - 1u); zr_rect_t tail; tail.x = (int32_t)x; tail.y = (int32_t)y; tail.w = (int32_t)(m->cols - x); tail.h = 1; return zr_fb_fill_rect(&m->painter, tail, &m->ts.style); }Then simplify both call sites:
if (!priv && intermediate == 0u && final == (uint8_t)'K') { const uint32_t mode = (param_count >= 1u) ? params[0] : 0u; if (mode == 0u && m->cols != 0u && m->rows != 0u) { - /* EL 0: erase from the cursor (inclusive) to the end of the line. */ - const uint32_t y = (m->ts.cursor_y < m->rows) ? m->ts.cursor_y : (m->rows - 1u); - const uint32_t x = (m->ts.cursor_x < m->cols) ? m->ts.cursor_x : (m->cols - 1u); - zr_rect_t tail; - tail.x = (int32_t)x; - tail.y = (int32_t)y; - tail.w = (int32_t)(m->cols - x); - tail.h = 1; - const zr_result_t rc = zr_fb_fill_rect(&m->painter, tail, &m->ts.style); + const zr_result_t rc = zr_vt_model_erase_cursor_to_eol(m); if (rc != ZR_OK) { return rc; } }And in
zr_vt_model_erase_below:static zr_result_t zr_vt_model_erase_below(zr_vt_model_t* m) { if (!m) { return ZR_ERR_INVALID_ARGUMENT; } if (m->cols == 0u || m->rows == 0u) { return ZR_OK; } - const uint32_t y = (m->ts.cursor_y < m->rows) ? m->ts.cursor_y : (m->rows - 1u); - const uint32_t x = (m->ts.cursor_x < m->cols) ? m->ts.cursor_x : (m->cols - 1u); - - zr_rect_t tail; - tail.x = (int32_t)x; - tail.y = (int32_t)y; - tail.w = (int32_t)(m->cols - x); - tail.h = 1; - const zr_result_t rc = zr_fb_fill_rect(&m->painter, tail, &m->ts.style); + const zr_result_t rc = zr_vt_model_erase_cursor_to_eol(m); if (rc != ZR_OK) { return rc; } + const uint32_t y = (m->ts.cursor_y < m->rows) ? m->ts.cursor_y : (m->rows - 1u); if (y + 1u >= m->rows) { return ZR_OK; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/zr_vt_model.c` around lines 807 - 824, The cursor-clamping + single-line-fill logic is duplicated in zr_vt_model.c (inside the branch handling final 'K' and in zr_vt_model_erase_below); extract this into a small static helper (e.g., zr_vt_model_erase_cursor_to_eol) that takes zr_vt_model_t* m, clamps cursor_x/cursor_y against m->cols/m->rows, constructs the zr_rect_t tail and calls zr_fb_fill_rect, returning its zr_result_t; replace both original blocks with calls to this helper and propagate/return any non-ZR_OK result from zr_fb_fill_rect.Source: Coding guidelines
src/core/zr_diff.c (1)
2610-2680: ⚡ Quick winSplit
zr_diff_render_commit()into smaller helpers.This new function is well over the repo’s 50-line cap for C functions. Extracting validation/setup, row emission, and final-state rebasing would bring it back under the guideline without changing behavior.
As per coding guidelines,
**/*.c: Functions should be 20-40 lines, with a maximum of 50 lines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/zr_diff.c` around lines 2610 - 2680, zr_diff_render_commit is too long; split it into three focused helpers without changing behavior: extract the initial validation and setup (the null/arg checks, ctx init, sb init, initial move and inline_rows_claimed) into a function like zr_diff_render_commit_init(ctx,out_buf,out_cap,content,caps,initial_term_state,live_cols), extract the per-row paint+finish loop (the paint_cols computation, loop calling zr_diff_render_span and zr_diff_commit_finish_row) into zr_diff_render_commit_paint_rows(ctx,content,live_cols), and extract the tail logic (claim fresh line, truncation check, rebasing cursor/flags, setting out_len/out_final_term_state) into zr_diff_render_commit_finalize(ctx,out_len,out_final_term_state). Call these three helpers from zr_diff_render_commit and keep all original checks/return codes and zr_diff_zero_outputs behavior exactly the same.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/zr_diff.c`:
- Around line 2591-2607: zr_diff_commit_finish_row currently emits SGR then EL
(zr_emit_el0_clear_line_tail) without ensuring the terminal cursor is
re-anchored after zr_diff_render_span may have cleared
ZR_TERM_STATE_CURSOR_POS_VALID; update zr_diff_commit_finish_row to explicitly
re-anchor the cursor to ctx->ts.cursor_x (or emit an absolute cursor position)
before calling zr_emit_el0_clear_line_tail, e.g. invoke the library helper that
emits an absolute cursor column (or set the cursor position/flags and call
zr_emit_cursor_absolute equivalent) so that ZR_TERM_STATE_CURSOR_POS_VALID is
true and the EL (CSI K) clears from the intended column rather than a drifted
terminal column. Ensure references to ZR_TERM_STATE_CURSOR_POS_VALID,
ctx->ts.cursor_x, zr_emit_el0_clear_line_tail, and zr_diff_render_span are
considered when making the change.
---
Nitpick comments:
In `@src/core/zr_diff.c`:
- Around line 2610-2680: zr_diff_render_commit is too long; split it into three
focused helpers without changing behavior: extract the initial validation and
setup (the null/arg checks, ctx init, sb init, initial move and
inline_rows_claimed) into a function like
zr_diff_render_commit_init(ctx,out_buf,out_cap,content,caps,initial_term_state,live_cols),
extract the per-row paint+finish loop (the paint_cols computation, loop calling
zr_diff_render_span and zr_diff_commit_finish_row) into
zr_diff_render_commit_paint_rows(ctx,content,live_cols), and extract the tail
logic (claim fresh line, truncation check, rebasing cursor/flags, setting
out_len/out_final_term_state) into
zr_diff_render_commit_finalize(ctx,out_len,out_final_term_state). Call these
three helpers from zr_diff_render_commit and keep all original checks/return
codes and zr_diff_zero_outputs behavior exactly the same.
In `@tests/unit/zr_vt_model.c`:
- Around line 807-824: The cursor-clamping + single-line-fill logic is
duplicated in zr_vt_model.c (inside the branch handling final 'K' and in
zr_vt_model_erase_below); extract this into a small static helper (e.g.,
zr_vt_model_erase_cursor_to_eol) that takes zr_vt_model_t* m, clamps
cursor_x/cursor_y against m->cols/m->rows, constructs the zr_rect_t tail and
calls zr_fb_fill_rect, returning its zr_result_t; replace both original blocks
with calls to this helper and propagate/return any non-ZR_OK result from
zr_fb_fill_rect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cc2a5c3-dc85-4a63-9de5-59d5fdb6a654
⛔ Files ignored due to path filters (2)
tests/golden/fixtures/commit_001_single_row/expected.binis excluded by!**/*.bintests/golden/fixtures/commit_002_two_rows_styled/expected.binis excluded by!**/*.bin
📒 Files selected for processing (23)
CHANGELOG.mddocs/VERSION_PINS.mddocs/abi/c-abi-reference.mddocs/abi/versioning.mddocs/modules/DIFF_RENDERER_AND_OUTPUT_EMITTER.mddocs/user-guide/rendering-model.mdexamples/inline_status.cinclude/zr/zr_config.hinclude/zr/zr_engine.hinclude/zr/zr_version.hsrc/core/zr_diff.csrc/core/zr_diff.hsrc/core/zr_engine.csrc/core/zr_engine_present.inctests/CMakeLists.txttests/golden/fixtures/commit_001_single_row/case.txttests/golden/fixtures/commit_001_single_row/expected.hex.txttests/golden/fixtures/commit_002_two_rows_styled/case.txttests/golden/fixtures/commit_002_two_rows_styled/expected.hex.txttests/golden/golden_commit_render.ctests/unit/test_engine_commit_scrollback.ctests/unit/zr_vt_model.ctests/unit/zr_vt_model.h
When a committed row ends on a drift-capable glyph (wide/non-ASCII), the tracked cursor position is invalidated and EL would erase from whatever column the terminal actually reached. Re-anchor to the painted edge with absolute-column CHA (columns are drift-safe) before the tail erase. Adds golden commit_004_wide_glyph_reanchors_tail pinning the CHA+EL bytes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Addressed in the latest commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/golden/golden_commit_render.c (2)
136-151: ⚡ Quick winReplace glyph-width/UTF-8 literals with named constants.
Line 143 (
3u), Line 145 (2u), and Line 149 (0u) encode important glyph semantics; promoting them to local named constants/macros will make this helper less brittle and easier to audit.As per coding guidelines, “Extract magic numbers to named constants with descriptive names instead of using bare literals.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/golden/golden_commit_render.c` around lines 136 - 151, In zr_fb_set_wide, replace the magic literals (3u, 2u, 0u) with descriptive named constants (e.g., WIDE_GLYPH_BYTES, WIDE_GLYPH_WIDTH, EMPTY_GLYPH_LEN) declared at the top of the function or as local static macros; update uses in memset/memcpy and assignments to lead->glyph_len, lead->width, cont->glyph_len and cont->width (and any memset sizes) to use those constants so the intent around glyph byte-length, display width and empty glyph state is explicit.Source: Coding guidelines
161-185: ⚡ Quick winAdd Arrange/Act/Assert section markers in this golden test.
This test is long enough to benefit from the required test-structure markers for readability and maintenance (especially around fixture setup vs render call vs assertions).
As per coding guidelines, “Add section markers in long functions (/* --- Section Name --- /) and in tests (/ --- Arrange/Act/Assert --- /, / --- Validate/Compute/Emit --- */).”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/golden/golden_commit_render.c` around lines 161 - 185, Add test section comment markers to the ZR_TEST_GOLDEN(commit_004_wide_glyph_reanchors_tail) test: insert a /* --- Arrange --- */ before the fixture/setup code (zr_fb_init, zr_fb_clear, zr_fb_set_* and initial state/caps/out buffer setup), a /* --- Act --- */ immediately before the call to zr_diff_render_commit, and a /* --- Assert --- */ before the assertions (ZR_ASSERT_TRUE checks and zr_golden_compare_fixture) and teardown (zr_fb_release). Keep markers as single-line C comments to improve readability without changing logic.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/golden/golden_commit_render.c`:
- Around line 136-151: In zr_fb_set_wide, replace the magic literals (3u, 2u,
0u) with descriptive named constants (e.g., WIDE_GLYPH_BYTES, WIDE_GLYPH_WIDTH,
EMPTY_GLYPH_LEN) declared at the top of the function or as local static macros;
update uses in memset/memcpy and assignments to lead->glyph_len, lead->width,
cont->glyph_len and cont->width (and any memset sizes) to use those constants so
the intent around glyph byte-length, display width and empty glyph state is
explicit.
- Around line 161-185: Add test section comment markers to the
ZR_TEST_GOLDEN(commit_004_wide_glyph_reanchors_tail) test: insert a /* ---
Arrange --- */ before the fixture/setup code (zr_fb_init, zr_fb_clear,
zr_fb_set_* and initial state/caps/out buffer setup), a /* --- Act --- */
immediately before the call to zr_diff_render_commit, and a /* --- Assert --- */
before the assertions (ZR_ASSERT_TRUE checks and zr_golden_compare_fixture) and
teardown (zr_fb_release). Keep markers as single-line C comments to improve
readability without changing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a48fc4d1-9554-454d-b15a-69077ef1d881
⛔ Files ignored due to path filters (1)
tests/golden/fixtures/commit_004_wide_glyph_reanchors_tail/expected.binis excluded by!**/*.bin
📒 Files selected for processing (4)
src/core/zr_diff.ctests/golden/fixtures/commit_004_wide_glyph_reanchors_tail/case.txttests/golden/fixtures/commit_004_wide_glyph_reanchors_tail/expected.hex.txttests/golden/golden_commit_render.c
✅ Files skipped from review due to trivial changes (2)
- tests/golden/fixtures/commit_004_wide_glyph_reanchors_tail/case.txt
- tests/golden/fixtures/commit_004_wide_glyph_reanchors_tail/expected.hex.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/zr_diff.c
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
1 participant
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.
Summary
Adds
engine_commit_scrollback()(engine ABI 1.3.0 → 1.4.0, library 1.4.0 → 1.5.0): stage drawlist-rendered rows for emission into terminal scrollback above the inline live region. This is the Ink<Static>/ Bubble TeaPrintlnprimitive — the missing piece for agent-style CLIs that stream finished content (chat messages, log lines, tool output) into terminal history while a live region keeps rendering below.Motivated by building a coding-agent TUI on Rezi; the companion Rezi PR exposes this as
app.printAbove(...).Design
viewport cols × rowsframebuffer — wrappers reuse their entire existing rendering pipeline; drawlist resources (DEF/FREE) act on a transient snapshot and never persist. No-partial-effects: a failed call stages nothing.zr_diff_render_commit): relative re-anchor to the region top, per-row paint with baseline-SGR tail erase (EL— BCE-safe) and LF claims so earlier lines scroll into history, then a fresh-line claim that becomes the new region top. Returns terminal state withSCREEN_VALIDcleared, so the live-region diff that follows re-establishes the inline baseline below the block — reusing the existing rebaseline machinery rather than inventing new repaint logic.ZR_COMMIT_ROWS_MAX(1024) per call,ZR_COMMIT_PENDING_ROWS_MAX(4096) total, 32 pending blocks;ZR_ERR_LIMITbeyond. INLINE mode only (ZR_ERR_UNSUPPORTEDin ALT).Test evidence
examples/inline_status.cnow commits checkpoint lines at progress quarters — verified live: committed lines stack in scrollback above the running region, survive app exit, and the prompt restores below the final frame.Compatibility
Checklist
README.mdand/ordocs/as appropriate)mkdocs build --strictpasses (docs changes)bash scripts/clang_format_check.sh --allpasses (formatting)🤖 Generated with Claude Code