Skip to content

fix(desktop): preserve long offscreen text in SnapShots - #11250

Merged
juliusmarminge merged 2 commits into
pingdotgg:mainfrom
Bil0000:t3code/fix-snapshot-a11y-visibility
Sep 12, 2026
Merged

fix(desktop): preserve long offscreen text in SnapShots#11250
juliusmarminge merged 2 commits into
pingdotgg:mainfrom
Bil0000:t3code/fix-snapshot-a11y-visibility

Conversation

@Bil0000

@Bil0000 Bil0000 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Long text below a scroll view could be collected by SnapShot but missing from the attachment details and the agent prompt. The structured tree silently cut names at 1,000 characters and values at 8,000, then took priority over the fuller plain text read.

This marks clipped text fields as truncated and reuses the existing plain text fallback when the structured tree is incomplete and the fallback recovers text. Complete trees keep their element bounds and state. A truncated tree also stays when plain text contains the same text, preserving useful metadata. If the plain text read fails, the partial tree remains available and is marked truncated.

Platform findings

Platform Finding and verification
macOS Reproduced with the real AX reader and an isolated Electron window on this Mac. A 151-line document exposed its final line to the native API, but the selected snapshot stopped at line 132. The same window now returns the final line without scrolling.
Windows The same text limits and selection code apply. Regression tests cover the Windows foreground-window path. Native UI Automation was inspected, but no live Windows desktop was available.
Linux The same code serves X11 and the GNOME, KDE, Hyprland, and Niri capture paths. Tests cover X11 and Wayland, including the existing desktop identity and coordinate cases. No live Linux desktop test was run. Portal/picker captures still omit accessibility data because they lack verified window identity.

The pinned xa11y Mac, Windows, and Linux readers enumerate children without filtering them by screen visibility. The loss fixed here happens after collection in T3's shared reader.

Verification

  • 276 focused tests passed across the snapshot reader, worker lifecycle, attachment details, and provider prompt suites. New regression cases failed before the fix.
  • Desktop typecheck, targeted lint, formatting, and diff checks passed.
  • Live macOS before/after capture used the same test window and the production snapshot reader. This was a reader integration test, not a full T3 client run.

Limits

The existing 32,000-character and three-second limits remain. A plain text fallback has no per-element coordinates. This preserves text the app exposes; it cannot recover content absent from the app's accessibility tree, such as unloaded items in a virtualized list. Existing saved snapshots are unchanged.

Implemented with GPT-6 in the native Codex harness.

Summary by CodeRabbit

  • Bug Fixes
    • Accessibility snapshots now accurately indicate when content or element trees are truncated.
    • Partial accessibility trees are preserved when complete text recovery is unavailable.
    • Long accessibility names, values, and descriptions are safely clipped while retaining useful tree information.
    • Off-screen invisible nodes correctly report unavailable bounds.
    • Long text outside scrollable areas remains represented consistently across platforms and sessions.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 11, 2026
@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Sep 11, 2026
Comment thread apps/desktop/src/snapShot/snapShot.ts
@macroscopeapp

macroscopeapp Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 256b87e

Macroscope's review found this PR approvable — This is a narrowly scoped desktop accessibility bug fix that preserves long offscreen text using an existing flat-text fallback while retaining structured metadata when appropriate. Existing limits and complete-tree behavior remain unchanged, with targeted coverage across supported platforms and failure modes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 3e087d1a-2449-4021-8bb8-d34d38148000

📥 Commits

Reviewing files that changed from the base of the PR and between 1bbfe8a and 256b87e.

📒 Files selected for processing (2)
  • apps/desktop/src/snapShot/DesktopSnapShot.test.ts
  • apps/desktop/src/snapShot/SnapShotAccessibility.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Accessibility snapshot generation now tracks truncation from long element fields, preserves usable rich trees during flat-text fallback, and reports combined truncation state. Tests add Linux, off-screen node, long-text, and flat-text recovery coverage.

Changes

Accessibility snapshot truncation and fallback

Layer / File(s) Summary
Truncation tracking and validation
apps/desktop/src/snapShot/snapShot.ts, apps/desktop/src/snapShot/snapShot.test.ts
boundedSnapShotString reports truncation through a callback. Tree traversal propagates this state. Tests cover oversized name, value, and description fields.
Accessibility result selection and platform coverage
apps/desktop/src/snapShot/SnapShotAccessibility.ts, apps/desktop/src/snapShot/DesktopSnapShot.test.ts
Rich element trees remain available when flat-text fallback is absent or matches the rich text. Returned truncation combines rich and flat states. Tests cover Linux, off-screen nodes, long text, and flat-text recovery failures.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 256b8

The accessibility snapshot changes preserve truncation metadata and select recovered flat text only when it differs from the rich tree. No current merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main fix: preserving long offscreen text in desktop SnapShots.
Description check ✅ Passed The description explains what changed, why it changed, platform behavior, verification results, and known limits. It omits the template headings and checklist, but the required information is mostly c…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@Bil0000

Bil0000 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

The docstring-coverage warning is advisory. This change adds no public API, and docstring-only edits are outside this focused bug fix. The changed behavior is covered by the regression tests, so I am leaving that style suggestion unchanged.

@juliusmarminge
juliusmarminge merged commit 57b23a0 into pingdotgg:main Sep 12, 2026
24 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 12, 2026
## What's Changed
* feat(settings): add open source license notices by @juliusmarminge in pingdotgg/t3code#8962
* perf(client): reduce repeated sorting and date formatting by @Bil0000 in pingdotgg/t3code#11019
* feat: add inline file previews and attachment chips across surfaces by @chrisdeeming in pingdotgg/t3code#11265
* fix(desktop): preserve long offscreen text in SnapShots by @Bil0000 in pingdotgg/t3code#11250
* perf(server): avoid workspace scans when loading pull requests by @Bil0000 in pingdotgg/t3code#11299


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260912.1576...v0.0.41-nightly.20260912.1599

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260912.1599
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 13, 2026
Merges `upstream/main` at `0c5771d60` into the fork, from merge base
`e81606494` — 32 upstream commits.

The range is mostly client polish, plus two structural changes that
mattered
here: upstream extracted the sidebar header into a new component
(`SidebarThreadHeader.tsx`, pingdotgg#11315), which is where two fork gates had
to be
re-homed, and upstream added a `context` field to orchestration messages
at the
exact anchor the fork's `origin` field sits on, which is four of the
eight
conflicts.

## Merge stats

- Landed (`HEAD^1..HEAD`): 489 files, 36117+/5930−
- Upstream range (base..`HEAD^2`): 484 files, 35784+/5824−
- Fork delta (`HEAD^2..HEAD`): 767 files, 78724+/2528−

The two file lists reconcile exactly. The 5 extra landed files are all
fork-owned and none of them is upstream work:
`apps/web/src/fork/SidebarThreadFilter.tsx`
(one className, described below), `docs/fork/inventory.json`,
`docs/fork/gaps.md`, `docs/fork/upstream-merge-log.md`, and
`.agents/skills/fork-upstream-merge/scripts/unsupported-methods.mjs`.
Nothing in
the upstream range failed to land.

## Conflicts

All 8 were resolved by the verdict `preflight.mjs` printed. No `decide`
conflict
was left unresolved.

- `projector.ts`, `orchestration.ts`, `threadReducer.ts`,
`MessagesTimeline.tsx`
— `converged — message-origin-upstream-files`, and all the same
conflict:
upstream appended where the fork already appends. Both sides kept, twice
per
file for the first three. `duplicate-adds.mjs` confirms no line was
taken
  twice.
- `Sidebar.tsx` — `converged — thread-visibility-upstream-files`. Took
upstream
whole; its `SidebarThreadFilter` import was left unused by the
extraction and
  was removed.
- `SettingsSidebarNav.tsx` — unlisted. Kept the fork's
`settingsPathEnabled`
filter over the personal nav items and took upstream's new active-state
rule
(`/settings/general` stays active on `/settings/open-source-licenses`).
- `ChatComposer.tsx` — unlisted, so `decide, then add an entry`. Both
fork deltas
  survived and the entry is now written; see below.
- `routeTree.gen.ts` — generated; regenerated with
`regen-route-tree.mjs` after
  the install.

`pnpm-lock.yaml` auto-merged rather than conflicting, so it was reset to
`upstream/main` and the fork edges re-derived with `vp i`. The remaining
diff
against upstream is exactly the `@t3tools/moatless-api` workspace link,
`mermaid ^11.17.2`, and one alchemy peer hash.

Two findings worth naming here:

- **A fork gate's host file was replaced by a file upstream had not
written yet.**
  pingdotgg#11315 extracted the whole sidebar header into
`apps/web/src/components/sidebar/SidebarThreadHeader.tsx`. Both fork
deltas
were re-applied there additively — the `FEATURES.projectManagement` gate
on
New project, and `<SidebarThreadFilter />` as a third child of
upstream's new
segmented icon well. No props threaded, no state added, no upstream JSX
re-indented. The one edit outside that file is
`SidebarThreadFilter.tsx`'s
  trigger className, now `size-7` so it matches upstream's own
  `SidebarHeaderIconButton` in the well it now sits in.
- **The unsupported-method derivation could not read the backend, and
that was
the script's fault, not a finding.** `unsupported-methods` exited 2 with
"could
not read the backend dispatch". The Moatless backend moved its dispatch
a
second time: `crates/t3code/src/rpc/dispatch.rs` is now a module stub
over an
`rpc/dispatch/` directory whose `routing.rs` holds the arms and whose
siblings
hold the handler bodies. `BACKEND_APIS` now names the directory and the
script
concatenates every `.rs` file in it — pointing it at `routing.rs` alone
would
have read the arms and lost the handlers, and `refusesInside` only
follows
calls it can find in the same source, so every conditional refusal would
have
  come back as a false DROP.

## Inventory

- `moatless-admin-pages` was stale: it still listed the two Workspaces
admin
routes that the 2026-09-12 commit folded into the project settings page.
Re-pointed to the five surfaces that remain, and the untracked delta
that move
  left behind is now its own entry, `project-workspace-settings`.
- `chat-surface-gates` gained
`apps/web/src/components/chat/ChatComposer.tsx`
with a guard on `FEATURES.accessMode`, plus a `chat-composer-gates` path
policy
so the next merge gets a cached verdict instead of the same decision.
The two
deltas there are the runtime-mode picker lifted into a
`runtimeModePicker`
  const behind the flag, and `phase === "running"` left out of
  `collapsedComposerPrimaryActionDisabled`.
- `inventory-check.mjs` is clean.

## Unsupported methods

0 ADD, 0 DROP, 2 KEEP (`git.preparePullRequestThread`, `vcs.switchRef`),
5 known
exceptions still firing, no stale ones. `packages/contracts/src/rpc.ts`
is
unchanged: the range's one unsupported-surface change is upstream's
Cursor
`--classic` launcher fix, which lands on a method already refused.

## Feature classification

### Usable as-is

Client-side work the fork can expose with no Moatless backend or
deployment
change. 28 of the 32 commits.

- Open-source license notices page (pingdotgg#8962) — new
`/settings/open-source-licenses`
route; upstream also made `/settings/general` stay active while it is
open.
- Client perf: fewer repeated sorts and date formats (pingdotgg#11019).
- Inline file previews and attachment chips across surfaces (pingdotgg#11265) —
rides
  `attachments.createUploadUrl` and `assets.createUrl`, both dispatched.
- Subagent spawns as an expandable work row (pingdotgg#11433) and those rows kept
visible
under folded turns (pingdotgg#11474) — derived from the orchestration event
stream the
  backend already serves.
- Opt-in thread notifications and sounds (pingdotgg#11481) — client settings,
persisted
  through the `server.getSettings` read the backend serves.
- Large pastes folded into text attachments (pingdotgg#11442); user input kept
outside
collapsed work (pingdotgg#11363); each chat message exposed as a heading for
screen
  readers (pingdotgg#11199); the default diff file state (pingdotgg#11484).
- Sidebar project scope folded into the search row (pingdotgg#11315); thread
status icons
completed and input threads kept prominent (pingdotgg#11461); sidebar search and
footer
  spacing (pingdotgg#11466); draft row heights matched to thread rows (pingdotgg#11512).
- Image chips tinted with their average colour (pingdotgg#11468); viewer controls
moved
outside the media with arrow navigation restored (pingdotgg#11470); snapshot
preview size
preserved in sent messages (pingdotgg#11429); preview focus preserved on window
return
  (pingdotgg#11444).
- Unavailable account limits made more visible (pingdotgg#10601) — web-only; the
backend
  dispatches `server.getUsageSummary`.
- Saved environments switched off instead of removed (pingdotgg#11478) — entirely
client-side (connection catalog and registry). This build runs one
environment
and gates the Connections settings page, so nothing on screen changes;
the
  catalog behaviour carries.
- Desktop and mobile: long offscreen text in SnapShots (pingdotgg#11250), native
preview
User-Agent kept for Turnstile (pingdotgg#7110), bounded backend shutdown wait on
quit
(pingdotgg#7599), expo-audio pinned (pingdotgg#11426), photo library picks rendered to a
bounded
JPEG off the JS thread (pingdotgg#11440), launch crash with a PR stack (pingdotgg#11486),
the
  shared-content alert after sending (pingdotgg#11487).
- Repository hygiene: `.pnpm-store/v11` deleted.

### Unsupported in Moatless / needs implementation

- **Cursor links open in classic IDE mode (pingdotgg#11498).** Upstream gave
Cursor
`baseArgs: ["--classic"]` in `packages/contracts/src/editor.ts` so a
file open
  targets the IDE rather than its Agents Window, and tested it in
  `apps/server/src/process/externalLauncher.ts`. The method behind it,
`shell.openInEditor`, is not dispatched — the browser is not on the
machine the
workspace is on — so this lands in the contract and in `apps/server` and
changes nothing here. Recorded in `docs/fork/gaps.md` under _Opening in
an
external editor_, whose standing conclusion is that the surface is a
candidate
  for deletion rather than for serving.

### Backend behavior to consider reproducing in Moatless

All three are recorded in `docs/fork/gaps.md` under _Runtime fixes
upstream made
to its own server_. Nothing in this repository holds them open; they are
Moatless-side work.

- **Listing pull requests should read only the projects asked about
(pingdotgg#11299).**
`listWorkspaceProjects` fetched the whole shell snapshot and filtered
it; it now
  asks the projection for the one project, or for the listed ids
  (`apps/server/src/pullRequest/PullRequestService.ts`,
  `persistence/Layers/ProjectionSnapshotQuery.ts`). Moatless dispatches
`pullRequests.summary`, so the same cost lands on it as soon as a
summary is
  derived from a list.
- **Usage should read each provider account's own history directory
(pingdotgg#11485).**
Upstream resolves an account's home from its home setting or its
`CODEX_HOME` /
`CLAUDE_CONFIG_DIR` / `GROK_HOME` variable, counts disabled accounts,
and
de-duplicates accounts sharing a directory
(`apps/server/src/usage/UsageService.ts`).
Moatless serves `server.getUsageSummary` itself, so an account with a
custom
home reports zero there — or double — until it resolves homes the same
way.
- **Forgejo and Gitea remotes should be first-class source control
(pingdotgg#11436).**
Upstream recognises both hosts and drives them with the `fj` and `tea`
CLIs
  across remote identity, PR creation and PR sync (`git/GitManager.ts`,
  `project/RepositoryIdentityResolver.ts`,
`orchestration/PullRequestSyncReactor.ts`). Moatless owns git and pull
requests,
so a Forgejo or Gitea project is an unrecognised host there regardless
of what
  the client can render.

## Verification

`verify.mjs` is green on seven of eight checks: `duplicate-adds` (none
across 34
files both sides changed), `tripwires` (3 deleted surfaces intact,
exactly the 5
known re-deletions, 3 allowed workflows), `resolution-check` (16
fork-delta paths
still differ from upstream, 17 carry upstream's change, 17
theirs-verbatim
byte-identical, 18 unlisted), `unsupported-methods`, `fmt:check`,
`lint`,
`typecheck`.

`test` is red on one file, and it is the standing environmental failure
rather
than a merge regression:

- `@t3tools/desktop` → `scripts/browser-secret-native.test.mjs > bundled
libsecret
helper` fails with `Package 'libsecret-1' not found` from `pkg-config`.
1 file
  of 105; the rest of the package is 1341 tests passed. The test file is
byte-identical to upstream, arrived on the fork before this merge, and
the
sandbox image ships neither `libsecret-1` nor its pkg-config file. There
is no
  root in the sandbox, so it cannot be installed here. Recorded in
`docs/fork/gaps.md` under _The desktop suite needs libsecret, which the
sandbox
  does not have_.

Four packages did not finish under `vp run -r test` and were each run
alone
again, all green: `@t3tools/mobile` (165 files, 1528 tests), `t3` (317
files,
4528 tests), `@t3tools/web` (412 files, 5205 tests), `t3code-relay` (30
files,
284 tests).

The owned-concern sweep over newly added upstream files found no keyword
hits, so
no `concerns` entry was needed.

**CI caught one thing no local check runs.** `Build & push moatless-t3`
failed on
the first push: upstream's new `t3code:third-party-licenses` plugin
(pingdotgg#8962) runs
in `generateBundle` and refuses any bundled package whose license it
cannot
resolve, and three packages reach the web bundle only through the fork's
own
`mermaid` edge — `khroma` via mermaid, `fastdom` and `strictdom` via
cytoscape
under it — so upstream's config has never carried overrides for them.
Fixed with
three `packageOverrides` entries: `khroma` needed a `license: "MIT"`
declaration
only, since it ships its own `license` file, and `fastdom` and
`strictdom` needed
a `generatedNotice` each, since both declare MIT and ship no notice
file.
Verified with the build itself — all three now appear in
`apps/web/dist/third-party-licenses.json` with a license and a notice,
and the
workflow is green. The delta is held by the `mermaid-diagrams` inventory
entry
plus a `third-party-licenses-config` path policy, and the reason it
escaped
`verify.mjs` — which has no build step at all — is now
`docs/fork/gaps.md`, _Nothing builds the web app before a merge is
pushed_.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants