Skip to content

Preserve painted transcript pages across saved-history appends - #1399

Closed
trissim wants to merge 4 commits into
mainfrom
fix/followed-paint-only
Closed

trissim wants to merge 4 commits into
mainfrom
fix/followed-paint-only

Conversation

@trissim

@trissim trissim commented Nov 1, 2026 •

Copy link
Copy Markdown

Summary

3 commits, 5 files, +284/-36. Current head 6e5c16f720ecccc5e072ea7710648ba631a952c7, rebased onto main 03f32f270ec4ba52dc9ba559b43c4246fd5f945a. Four production owners and presentation-fencing controls are separated from #1393 so the usage repair can ship without the physical-run harness. The first commit mechanically preserved the paint blobs; the second repairs the acquisition/publication lifetime; the third corrects test ingress and establishes actual saved paint before freezing its source. The two reported serial failures remain unverified at this new head. No harness/runtime-package change is included.

What changes for the user

Following saved transcript output extends the already-painted prefix instead of replacing it. A known undrawn destination must actually publish before live output can retire. Failed append acquisition forces replacement publication, including when a rewritten file reuses its path and grows past the old byte offset. A held reader keeps its paint on a successful read. Saved-history height is not live/native prune pressure.

Owner and consumer change

Before:

  • FollowTailCommit.extend acquired a replacement and republished preceding pages.
  • Access-only success could retire live output before the known destination painted.
  • Cursor-after acquisition gave ClosedHistory an incomplete replacement.
  • Conversation prune pressure counted bound saved history as live/native output.
  • The first version caught TranscriptChanged inside PreparedAppendCommit, read a complete snapshot, then independently selected append publication again. Filename/offset containment cannot recover that discarded acquisition failure.

After:

  • PreparedAppendCommit inherits PreparedPageCommit's capture/source-cut/current/draw evidence and owns cursor-after acquisition. PreparedPageCommit owns complete acquisition/replacement; PreparedCommit owns capture without replacement paint.
  • Each source's declared commit_preparation supplies acquisition capability. CommitPlan captures the corresponding publication member before that read. Its existing viewport family chooses retaining versus following behavior; a TranscriptChanged fallback fixes that same read's member to PreparedPageCommit. Later capture consumes the retained member rather than reselecting from the source. No read-mode flag, wrapper family, controller or type roster.
  • Shared PreparedCommit.capture replaces the two viewport-specific extend implementations. Existing cursor containment still selects replacement for a changed filename or rewind; the retained failure decision additionally covers a same-path rewrite whose new cursor contains the old position.
  • TranscriptSourcePreparation.publish_append shares bounded extension and requires current publication, no known gap and the actual requested endpoint before destructive retirement. Weaker access-only consumers retain their original contract.
  • Live pressure derives from actual layout minus bound saved-history height; no mirrored counter.

The source-cut/current/ready/lock/commit/retirement guards remain. The genuine Textual self-wait correction landed separately in #1394 and is not duplicated.

Review53818: complete acquisition and held readers

ClosedHistory genuinely reaches CommitPublication, including while the viewport is held. Complete acquisition is deliberate: coverage for a retained-only prefix cannot depend on an absent saved reader. A successful held-viewport commit still captures PreparedCommit and keeps its original pages, fragments and reader position; complete acquisition does not mean replacement paint. The existing closed-source case now covers both viewport intents.

ProjectedTranscriptSource is different: the only production constructor is LineHistory.projected_source (widgets/transcript_history.py:862), consumed by SeekingFilter.advance (transcript_filter.py:197) to mount ProjectedTranscriptHistory as the filter overlay. It is not the BoundView's ConversationHistory source selected by CommitPublication. This is the bounded named-consumer trace, not a claim that arbitrary dynamic callers were proved absent. The default complete-read method is not a new projected-overlay commit path.

CommitPlan's stale viewport-only acquisition docstring is corrected in the same commit.

Source evidence

Existing NRA SourceModule/ModuleSyntaxIndex/CompactClassFamilyIndex map: 35modules,354original classes,353canonical classes,1unprojected OPEN,0parse omissions before and after; syntax sites17107→17173. All354 declaration/lexical-owner/base/decorator rows remain equal in traversal order. No classes or inheritance edges added. Raw declarations/writes/decisions/imports/consumers are retained in platform-source/paint-review-family-{before,after}.json in the owned Core checkout. Direct-name retrieval spans both repositories' src/tests/tools, with named Core/Textual dependencies. Aliases, dynamic binding, effects and arbitrary movement remain OPEN, not certified by this map.

Two changed files compile; three actual Toad modules import from this source using installed0402 Core in the read-only0402/3475 dependency prefix. Actual MRO/import origins and the declaration comparison passed rc0 in pi-ui-performance/paint-review-qualify-02.log. No local NRA gate, pytest or physical run is represented by this source check.

Executed app proof and outstanding counterexamples

The original five affected app cases collected5/passed5,71.91s,rc0 at26a application/fixture + installedceeed Core/Textual. Artifacts: /home/ts/.cache/agent-scratch/pi-ui-performance/dm-app-controls-03/{pytest.log,pytest.rc,origins.log}. They covered painted-page/fragment retention, the actually admitted known-gap destination, complete closed-source prefix and both live-pressure variants. Earlier setup5fail and4pass/1fail remain failed. No production method patch, guard weakening or restored Thread.project/Session details API.

Review54167 executed bf87092 against current-main dependencies: Claude reports 12 passes, two stale TranscriptChanged imports, two real serial failures, and six contention-only failures that pass serially. The serial failures were the held closed-prefix marker disappearing and a followed commit returning with drawn_end_more still True.

Latest test-only correction:

  • Import TranscriptChanged from its owning coordination_errors module.
  • Before stopping observations/suspending the source, require the saved history itself to paint the marker with source_ready and reports_coverage. A live replay showing identical text is not this precondition. The shared helper serves append, gap, closed-prefix and rewrite cases.
  • Keep the actual captured gap publication for failure diagnostics: current publication/viewport, source readiness, follow intent, dirty/commit flags and actual cursors. The original gap and paint assertions remain intact.

No production guards or family declarations changed in this checkpoint. The file compiles; the latest setup correction is UNRUN and is not claimed to fix either serial failure. Commander owns the necessary combined selection, including the two original failures and both real rewrite/import cases. The original five passes are not current-head acceptance.

Headless app results do not prove configured-native read/edit arrivals, warm M-view paint/selection/draft/undo, hidden/reopen/tab-return, full physical performance, installation or original P867 coordinates. I retain implementation and verification ownership through the combined installation. Commander owns integration/install; auto-merge remains off pending exact-head review.

Separate work

#1393 retains the physical lifetime/observation overhaul and explicit module-entry fix; #3825 retains exact-input runtime/interpreter/archive evidence. The prepared0402/3475 prefix remains unchanged and borrowed read-only. Its preparation/readback and fresh stopped-history copy do not establish this revised paint head's live acceptance.

Assumed: the original five app results retain26a/ceeed scope; Review54167 retains its bf87092 dependency scope; the latest setup correction and two serial failures still need execution; the earlier NRA/import map is not proof of all current-base dynamics; current combined dependencies, installation and configured read/edit remain to be verified.

Copilot AI balanced review requested due to automatic review settings November 1, 2026 23:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim force-pushed the fix/followed-paint-only branch from 6e5c16f to 674b014 Compare November 2, 2026 04:10

trissim commented Nov 2, 2026

Copy link
Copy Markdown
Author

Closed in Tristan's board clear (2026-11-02): superseded by #1393, which carries the retained followed paint. The branch is kept.


Generated by Claude Code

@trissim trissim closed this Nov 2, 2026
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.

2 participants