Skip to content

test(pty): speed up integration synchronization - #1098

Open
benvinegar wants to merge 14 commits into
mainfrom
autoresearch/pty-tests-2026-09-10
Open

test(pty): speed up integration synchronization#1098
benvinegar wants to merge 14 commits into
mainfrom
autoresearch/pty-tests-2026-09-10

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Problem

The serial PTY integration suite spent substantial time waiting for generic terminal idleness before checking the concrete state that each interaction was meant to produce. Repeated input loops and post-exit daemon cleanup added further deterministic overhead.

On the same Linux machine, the unchanged baseline completed in 209.05s.

Approach

  • add guarded destination-driven key, text, click, and hover helpers that reject already-satisfied predicates before sending input
  • preserve Tuistory click targeting and SGR input parity with focused helper coverage
  • batch repeated horizontal keys and shifted-wheel events only where tests assert the final state and preserve every input event
  • remove duplicate idle waits where the preceding interaction already completed the same contract
  • gate retained file-view polling on an unambiguous stream-mounted marker while preserving its original readiness timeout
  • stop polling for new daemon metadata after signal-test children have already exited, while still terminating every recorded daemon
  • retain documented startup, paging, parser, expansion, and multi-commit waits

This changes test orchestration only. It does not change product behavior, reduce fixture complexity, run PTY files concurrently, shorten safety windows, or remove assertions.

Results

  • full serial PTY suite: 209.05s → 158.88s
  • improvement: 50.18s / 24.0%
  • coverage: 176 passed, 1 platform skip
  • same-machine held-out origin/main control reproduced at 209.26s

Promising changes were confirmed with repeated complete-suite samples. The final signal-cleanup sample passed the complete suite and backpressure checks; its subsequent unchanged confirmation runs were interrupted before completion.

Validation

  • ./.auto/measure.sh (runs unchanged bun run test:integration serially)
  • bun test test/pty/harness.test.ts
  • bun run typecheck
  • bun run lint
  • git diff --check origin/main...HEAD

No visual evidence is included because the patch changes test synchronization and cleanup only, not terminal UI behavior. Testing was performed on Linux; Windows-only signal paths remain skipped as before, and macOS was not tested.

Residual risk

The raw no-idle click helper intentionally mirrors the pinned Tuistory targeting behavior. Its parity should be rechecked when Tuistory is upgraded.

This PR description was generated by Pi using GPT-5.6 Sol

…pshots

Result: {"status":"keep","pty_suite_ms":206306.11,"pty_reported_ms":206290,"pty_pass_count":174,"pty_skip_count":1,"slowest_test_ms":8707.58,"tests_over_3000ms":13}
Result: {"status":"keep","pty_suite_ms":198799.95,"pty_reported_ms":198790,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":8630.48,"tests_over_3000ms":13}
Result: {"status":"keep","pty_suite_ms":191793.75,"pty_reported_ms":191780,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":8622.65,"tests_over_3000ms":12}
…save transitions

Result: {"status":"keep","pty_suite_ms":191780.54,"pty_reported_ms":191770,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7932.46,"tests_over_3000ms":12}
Result: {"status":"keep","pty_suite_ms":188144.25,"pty_reported_ms":188130,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7716.47,"tests_over_3000ms":11}
Result: {"status":"keep","pty_suite_ms":185210.1,"pty_reported_ms":185200,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7727.25,"tests_over_3000ms":10}
Result: {"status":"keep","pty_suite_ms":178810.81,"pty_reported_ms":178800,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7713.9,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":176492.9,"pty_reported_ms":176480,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7723.4,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":174952.26,"pty_reported_ms":174940,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":8221.26,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":171895.54,"pty_reported_ms":171880,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7721.06,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":171351.89,"pty_reported_ms":171340,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":8223.49,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":168603.75,"pty_reported_ms":168590,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7719.79,"tests_over_3000ms":8}
Result: {"status":"keep","pty_suite_ms":164754.34,"pty_reported_ms":164740,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":7727.59,"tests_over_3000ms":7}
Result: {"status":"keep","pty_suite_ms":158876.9,"pty_reported_ms":158870,"pty_pass_count":176,"pty_skip_count":1,"slowest_test_ms":8221.56,"tests_over_3000ms":7}
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
hunk-web Ignored Ignored Sep 11, 2026 12:48pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR accelerates serial PTY integration tests by introducing destination-driven input helpers, batching repeated input, tightening retained-view polling, and simplifying post-signal daemon cleanup.

  • Adds guarded key and click transition helpers that require destinations to be absent before input.
  • Replaces many generic terminal-idle waits with waits for concrete rendered outcomes.
  • Batches repeated keyboard and shifted-wheel input while retaining final-state assertions.
  • Adds focused synchronization-helper tests.
  • One hover helper still needs synchronization that ties readiness to the current candidate move.

Confidence Score: 4/5

The PR is not yet safe to merge because hover probing can acknowledge a stale affordance and target the wrong review row.

The new hover path sends successive mouse moves without waiting for each move to settle, while readiness accepts any visible add-note badge; delayed rendering can therefore associate a successful probe with the preceding candidate row.

Files Needing Attention: test/pty/harness.ts

Important Files Changed

Filename Overview
test/pty/harness.ts Adds destination-driven key/click synchronization and batched-input utilities, but removes necessary per-candidate hover settling.
test/pty/harness.test.ts Covers key-transition completion, pre-satisfied destination rejection, and no-retry behavior.
test/pty/lifecycle.test.ts Simplifies daemon cleanup after signal-test children have exited.
test/pty/file-views-integration.test.ts Uses destination-driven transitions and an additional stream-mount marker for retained previews.
test/pty/layout.test.ts Batches horizontal keyboard and shifted-wheel events while preserving final-state checks.
Prompt To Fix All With AI
### Issue 1
test/pty/harness.ts:185-188
**Stale Hover Badge Accepted**

After sending a mouse move, this check accepts any visible `[+]` badge without confirming that the current move has settled. When the helper advances between `row`, `row - 1`, and `row + 1`, a badge from the previous candidate can satisfy the wait. The caller can then click the stale badge coordinates and create a note on the wrong line. Please wait for the current move to settle or verify that the badge is on the intended row.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "Stop checking for never-created daemon m..." | Re-trigger Greptile

Comment thread test/pty/harness.ts
Comment on lines +185 to 188
sendMouseMove(session, x, y);
try {
await session.waitForText(/\[\+\]/, { timeout: 200 });
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Stale Hover Badge Accepted

After sending a mouse move, this check accepts any visible [+] badge without confirming that the current move has settled. When the helper advances between row, row - 1, and row + 1, a badge from the previous candidate can satisfy the wait. The caller can then click the stale badge coordinates and create a note on the wrong line. Please wait for the current move to settle or verify that the badge is on the intended row.

Prompt To Fix With AI
This is a comment left during a code review.
Path: test/pty/harness.ts
Line: 185-188

Comment:
**Stale Hover Badge Accepted**

After sending a mouse move, this check accepts any visible `[+]` badge without confirming that the current move has settled. When the helper advances between `row`, `row - 1`, and `row + 1`, a badge from the previous candidate can satisfy the wait. The caller can then click the stale badge coordinates and create a note on the wrong line. Please wait for the current move to settle or verify that the badge is on the intended row.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

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