Repository navigation
Preserve live reader position through tab return - #1419
Merged
Merged
Conversation
There was a problem hiding this comment.
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
force-pushed
the
fix/1418-reader-tab-return
branch
from
November 2, 2026 19:13
72aeebb to
b67ec21
Compare
|
debt-src at c045ad6 passes. |
trissim
force-pushed
the
fix/1418-reader-tab-return
branch
from
November 2, 2026 19:57
502e078 to
cf933e5
Compare
trissim
force-pushed
the
fix/1418-reader-tab-return
branch
from
November 2, 2026 21:11
cf933e5 to
d4d9e9a
Compare
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
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
Five commits, six files, +126/-12 on one reader-return issue. Current head
c045ad6313170c3bee0c3f9c2400e6073b780d56includes the source correction for the prior hosted gate's three findings and rebases onto current main. The first commit repairs the shared frame fixture and adds the row control; the second implements native-holder lifetime requirements through the existing resource budget; the third covers pending End's retained pager identities; the fourth removes redundant mounted-state validation and an unused base release declaration; the fifth supplies the already-admitted native Screen to the requirement query, resolving its typed Conversation from that screen rather than dereferencing an optional stored widget. No new cache, retention ledger or tab-specific scroll reset.Tracks #1418. Combined affected UI execution passed9/9 on exact
502e078d878fee3cb0cf567b4f25552097c594c7. The fourth and fifth commits' deltas are detailed below; no new9-case run is claimed. Corrected-head source judgment, hosted green and installed physical acceptance remain open. These admitted controls do not retrospectively certify every original field observation.Before / after
Before:
BlockPositionandArrivalPositionretained native Widget/page identities.ParkedPositionpreserved those holders while a tab was hidden.RetainedSessionViewStatethen restored that old holder into a rebuilt view. Geometry cannot place a detached old widget.After:
NativeReaderHoldersupplies the lifetime requirement once.BlockPosition(NativeReaderHolder, OffsetPosition)andArrivalPosition(NativeReaderHolder, OffsetPosition)compose native identity with their existing offset behavior.LatestPosition(NativeReaderHolder, HeldReaderDestination, TailPosition)composes the same requirement with pending End: itssourcesretain actual pager identities whose advance cannot be observed through an evicted old tree.UnreachedPositioncarries the wrapped position's requirement;ParkedPositioninherits it. Source-addressedFragmentPosition,RowPositionandTailPositionretain the default empty requirement.NativePresentationdeclares shared costs and the required-resource query with concrete empty-resource defaults. BothOperationalSessionPresentationandSessionThreadSidebarinherit it; duplicate sidebar zero declarations are deleted. Their actual release operations remain on the concrete lifetime owners; the shared budget value does not import MainScreen or declare an unused release no-op.NativeSessionSurfaceincludes those resources in the existingPresentationBudget.fitcall. The normal cost accounting remains intact; there is no competing store or silent raw-offset fallback.PresentationBudgetalready permits explicitly protected resources to exceed its nominal budget: source data is not discarded to satisfy a widget budget. A native holder is now correctly one of those protected requirements. Saved-key and tail positions remain evictable and reconstructable. A native-holder reader must release that requirement through the existing position lifecycle before its tree can be discarded.Frame fixture
RecordedHistoryScreenpreviously iteratedWorkspaceScreen.CSS_PATHdirectly. After1412 changed that declaration from a list to a string, the fixture tried to load its characters (screens/m). It now uses Textual's existing_css_path_type_as_listboundary normalization and resolves relative to the declaring WorkspaceScreen. DerivedTopRowScreenreceives the same corrected path set.Authored behavior controls
The reader control uses the existing real wheel route to move inside a response below a thought. It opens another session and returns, retaining named publication observations. It compares the actual row only in frames where that window is shown: painting another tab is not a preservation failure.
The two admitted policy cases are the ordinary budget and real budget pressure (
minimum_widgets=1,widgets_per_row=0). They invoke the existing reconciliation owner with actual widget costs, not a replacement eviction method or fabricated counts. Both require the native reader's original tree and its row to survive. Existing saved-tab eviction/editor-document/undo/draft restoration remains the counterexample to blanket retention.Evidence, including failures
screens/m. Its patch and private root remain retained./home/ts/.cache/agent-scratch/pi-commander/reader-502e-selection-01/{pytest.log,result.json}. No duplicate holder pytest/NRA/full-suite run was launched; prepared63 remains unlaunched. This is headless affected-path execution, not installed physical proof.Evidence:
/home/ts/.cache/agent-scratch/pi-ui-performance/reader-return-{61,62}/andreader-holders-{67,68,72}/. Earlier runs keep their original Core5f12/source-patch scope; later installations are not assigned retroactively.Hosted source correction
The cf933 hosted NRA epoch failed with NoneAbsence +1 in session_presentation.py and StringIdentity +2 / TypeCheckingImport +1 in presentation_window.py. Those failed results remain retained. The correction deletes my second mounted-state assertion: MainScreen.retained_native_presentations already admits only non-None conversations, and NativeSessionSurface collects and queries the admitted owners synchronously before any eviction await. Both admitted classes implement evict; the empty base evict had no caller, so deleting it also deletes the circular MainScreen dependency. Python3.14's native deferred annotation resolves the unquoted NativePresentation return type; finite AST extraction/get_type_hints verified that exact declaration without importing the application. No reader retention or resource accounting was removed. Epoch87 publication/rebase/push completed rc0 with remote=HEAD; no local NRA or pytest rerun.
The d4d9 hosted epoch caught a new union-attr error after assertion removal: widget's declared Conversation | None type still permitted absence. That failed result is retained in reader-gate-90.log. The fifth commit changes required_presentations to receive the existing admitted Screen and resolves its actual typed Conversation through native query_one; both resource families share the same Screen contract/default. The budget passes the screen already paired with each resource. No optional-state cast, extra absence probe, cache, duplicate membership or circular MainScreen type dependency is added. The new affected risk is resolving the hidden mounted screen's own conversation under ordinary and pressured reconciliation; the two interior-return cases specifically exercise it. Existing9/9 remains evidence for502e, not a claimed rerun of this corrected head.
Remaining acceptance
Assumed:1408's body is a report/sketch, not an executed certificate for this head. Existing old-window captures remain retained, not relabeled as installed1393 acceptance. No new stable source identity is fabricated for a live widget.