Repository navigation
Move stale tests to current API owners - #1169
Merged
Merged
Conversation
trissim
marked this pull request as ready for review
October 23, 2026 21:48
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.
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.
This was referenced Oct 23, 2026
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.
These tests still called APIs that their owners renamed or removed. Update them directly and delete the duplicate Core coordination-snapshot test: 8 insertions, 25 deletions across five test files. No source changes; the welcome-view tests and retirement failures remain evidence only.
Changes at d30fd1e:
Validation: no tests run. This command produces no output (exit 1):
git grep -nE "coordination_snapshot|claim_scope|navigation_target_name|transcript\.snapshot\(|history\.revision" -- testsThe channel, DM and feed constructors correspond to ChannelTarget, DirectTarget and FeedTarget in navigation_target.py:151-174; the screen uses the same HistoryRoute declarations through :114-116. UI equality assertions await the review run; no screen comparison was executed here.
Read-only findings (Toad source base 96ab9fe, Core display/observation declarations read at 01aa67d87; existing failure log /home/ts/.claude/jobs/47220aa9/tmp/drift-run.log):
B1(a): Core ViewReading.read (ui_model/observation.py:873-879) emits ViewsRetired. Toad CoordinationAccess.views_retired (comms_root.py:464-466) calls SessionNavigation.views_retired (session_navigation.py:209-229), which schedules close_many at :227 rather than removing the view synchronously. SidebarObserved independently replaces references at comms_root.py:411 and presents at :412. A paint can therefore still reach the deleted peer through workspace_sessions.py:232. close_many also awaits tab_order.close at :446 before deleting membership at :449-453, and awaits return_to at :459 before removing the mounted view at :462. TargetTree.session_selected still resolves the previous mode at widgets/sidebar_tree.py:902-905. The existing KeyError stack is specifically a paint: selection.py:66 -> comms_sidebar.py:195 -> sidebar_tree.py:877 -> workspace_sessions.py:130,232 -> screens/comms.py:92 -> navigation_target.py:116 -> conversation_kind.py:247 -> Core display_references.py:218. It does not enumerate every mounted view.
B1(b): All three direct CurrentDisplayReferences.thread callers in Toad:
Core's other .thread(...) call (history_views.py:465) is a different target method, not CurrentDisplayReferences.thread.
B1(c): The log records separate futures, not one serial failure stack. The self-wait is transcript_source_preparation.py:406-415 (cancel_node then join every worker); WorkingTranscript.execute calls source_failed within its worker at transcript_state.py:405-407, and source_failed retires at transcript_source_preparation.py:132. The closed-transcript failure is close_many :459 -> HistorySessionEntry.return_to :298 -> workspace_sessions.select :204 -> MainScreen._prepare_content :333 -> native.activate (session_presentation.py:450) -> acquire :374 -> TranscriptPresentation.bind :789. close() sets closed at transcript_publication.py:934; bind refuses it at :788-789. The saved log does not identify which close() invocation closed that owner, so it does not prove these are one retirement operation or that the self-wait caused the closed transcript. App teardown can also close all owner presentations at workspace_sessions.py:259-263.
B2:
No last-passing run SHA was supplied. These are the source commits that changed the assumptions, not invented claims that a historical test run passed. The welcome tests moved from tests/app to tests/textual in 8da6f2b after those changes.
Assumed: the supplied drift-run.log is the run described in the order; source-only inspection cannot recover an unlogged focused widget, the exact stalled startup await, or the closer of the owner transcript. The reviewer runs the six files. No section-B edits.