Skip to content

Add typed append-only scrollback output - #54

Merged
zikolach merged 6 commits into
mainfrom
docs/append-only-output-api
Aug 2, 2026
Merged

Add typed append-only scrollback output#54
zikolach merged 6 commits into
mainfrom
docs/append-only-output-api

Conversation

@zikolach

@zikolach zikolach commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • add bounded, callback-completed TUI.appendToScrollback output for detached typed components
  • require a running normal-screen TUI using NormalResizeClearPolicy.PreserveScrollback with a committed retained frame
  • preserve the existing non-waiting contended/reentrant flushRender() contract
  • render through a restricted, revocable context and fairly retry only unpublished resize-invalidated candidates
  • validate typed controls, reject destructive/cursor authority, remap Kitty IDs, and retain a bounded redacted ownership ledger
  • relocate and redraw the retained frame while restoring hardware cursor and coordinate-aware mouse origin
  • retain exactly-once append completions across stop and finite Cleaning cutoffs
  • provide shared JVM/Scala Native contract tests, JVM PTY ordering/restoration coverage, diagnostics, docs, and manual emulator smoke steps

OpenSpec

  • archived as openspec/changes/archive/2026-08-02-add-append-only-output-api/
  • promoted the new append-only-output specification and the updated component-rendering and terminal-runtime requirements
  • all 34 implementation tasks are complete; no active changes remain
  • strict validation passes for all 14 promoted specifications

Validation portability dependencies

Three adjacent changes are intentionally included because they were required to validate the append implementation on every promised surface:

  • build.mill supports SIGLYPH_NATIVE_CLANG / SIGLYPH_NATIVE_CLANGPP so all Native modules and tests can use Clang 16+, avoiding the older-toolchain runtime defects warned about by Scala Native.
  • TerminalInputParser keeps fixed sequences in a validated vector rather than eagerly building an immutable hash trie. Full Native concurrency testing exposed unstable first-use hash-trie initialization on a callback thread; the vector keeps initialization stack-bounded while preserving parser behavior.
  • scripts/test-terminal-pty.sh invokes BSD script(1) directly on macOS because it does not support GNU -e/-c. This is required for the new append PTY conformance test to run in macOS CI while preserving child exit status.

These changes add no runtime dependency or new terminal-control authority.

Validation

  • GitHub CI: Test and package jars — passed
  • GitHub CI: macOS terminal conformance — passed
  • mill core.test
  • mill coreNative.test.testOnly scalatui.core.AppendOutputSuite scalatui.core.TUIConcurrencySuite with Clang 22
  • mill __.compile
  • scripts/test-terminal-pty.sh
  • mill scalafmtCheck
  • mill scalafixCheck
  • openspec validate --all --strict — 14 passed, 0 failed
  • git diff --check

zikolach added 4 commits July 28, 2026 23:40
Add bounded callback-completed append work with typed-control validation, retained-frame relocation, Kitty identity isolation, lifecycle-safe completion, and shared JVM/Native coverage.
@zikolach
zikolach marked this pull request as ready for review August 2, 2026 05:14
@zikolach

zikolach commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

AI-assisted review feedback

I reviewed the latest implementation at a384bf0, including the OpenSpec artifacts, append admission/lifecycle logic, fairness scheduling, resize retry, restricted rendering context, callback completion, Kitty ownership isolation, retained-iTerm2 handling, tests, and CI results.

The implementation addresses the earlier architectural concerns: six-category fair scheduling, the bounded 64-operation queue, collision-safe Kitty ID allocation, permanent restricted-context revocation, resize retry yielding, serialized lifecycle completion, and retained-iTerm2 rejection. I did not find a core append-path correctness blocker, and both current CI checks plus strict OpenSpec validation are green.

A few cleanup items remain before merge:

  1. PR metadata is stale. The title still says “Propose,” while this PR now implements the API. The body also says implementation is deferred and reports 32 tasks, but tasks.md contains 34 completed tasks. Please update the title/body and validation summary.
  2. Please split or justify unrelated changes. The Native compiler/toolchain selection in build.mill, the TerminalInputParser lookup representation change, and the PTY script portability adjustment are not covered by the append-only output contract. They would be easier to review in a separate PR, unless they are required here and that dependency is documented.
  3. Avoid direct terminal writes in the new README callback example. println/System.err.println from the serialized TUI callback owner can disturb the retained live frame. Prefer demonstrating application-state updates or another TUI-owned presentation path.
  4. Replace the fixed Thread.sleep(20) in the capacity/backpressure test with a latch or bounded condition probe to reduce timing sensitivity.

Overall, the append-only implementation itself looks coherent and thoroughly covered; the remaining feedback is primarily merge-readiness, scope, documentation, and test robustness.

Disclosure: this feedback was produced with AI-assisted code review and verified against the current PR diff and CI state.

@zikolach zikolach changed the title Propose append-only typed output API Add typed append-only scrollback output Aug 2, 2026
@zikolach

zikolach commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed the merge-readiness feedback in 89c5196:

  • updated the PR title/body, OpenSpec count, validation summary, and documented why the Clang override, Native parser representation, and macOS PTY invocation are required for this PR’s promised validation surfaces;
  • changed the README callback example to update a retained Text component and request a TUI-owned render instead of writing directly to stdout/stderr;
  • replaced the fixed Thread.sleep(20) with start/completion latches and bounded awaits;
  • added in-code rationale beside the Native toolchain, parser initialization, and BSD script(1) paths.

Validation: focused JVM append suite, focused Scala Native append suite with Clang 22, all-module compilation, Scalafmt, Scalafix, strict OpenSpec validation (14/14), and git diff --check all pass.

@zikolach

zikolach commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

AI-assisted re-review

Rechecked the PR at 89c5196. The updated title/body and 34-task count are correct, the adjacent Native/parser/PTY changes now have explicit rationale, the README example stays within TUI-owned presentation, and the latest CI plus strict OpenSpec validation are green. I also verified locally that the macOS BSD script(1) invocation propagates a failing child exit status.

Two merge-readiness items remain:

  1. The backpressure regression is still timing-dependent. The new overflowStarted latch fires before appendToScrollback, and the assertion only proves that the thread did not finish within 100 ms. If that thread is descheduled after counting down the latch, a broken non-blocking implementation can still pass. Please wait with a bounded condition until the worker demonstrably reaches the blocked/waiting path, or add a deterministic test synchronization point before releasing the owner.
  2. The completed OpenSpec change remains active. add-append-only-output-api reports all 34 tasks complete and strict validation passes. Per the repository workflow, archive/promote it before treating the change as finalized, unless archival is intentionally planned as an explicit follow-up PR.

I found no new core append-path correctness issue.

Disclosure: this re-review was AI-assisted and verified against commit 89c5196, the current PR metadata, CI state, and strict OpenSpec validation.

@zikolach

zikolach commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Addressed the re-review items in 5392a8e:

  • the backpressure test now waits until Thread.getState demonstrably reaches WAITING inside the full-ingress condition path before releasing the drain owner; the existing completion latch still bounds the wake/finish assertion. This passes on both JVM and Scala Native.
  • synced the append deltas into promoted append-only-output, component-rendering, and terminal-runtime specifications, then archived the complete 34-task change at openspec/changes/archive/2026-08-02-add-append-only-output-api/.
  • updated the PR body to report the archive and promoted spec state.

Validation: focused JVM and Clang-22 Scala Native append suites, Scalafmt, Scalafix, strict OpenSpec validation (14 promoted specs), openspec list --json (no active changes), and git diff --check all pass.

@zikolach
zikolach merged commit 366a8cc into main Aug 2, 2026
2 checks passed
@zikolach
zikolach deleted the docs/append-only-output-api branch August 2, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant