Skip to content

test(mobile): make diff grammar assertions independent of wall time - #610

Merged
yaacovcorcos merged 1 commit into
mainfrom
codex/ci-flakes-2-20261020
Oct 19, 2026
Merged

yaacovcorcos merged 1 commit into
mainfrom
codex/ci-flakes-2-20261020

Conversation

@yaacovcorcos

Copy link
Copy Markdown
Contributor

What Changed

Test-only. In apps/mobile/src/features/diffs/nativeReviewDiffHighlighter.test.ts, the real-Shiki test wrapper now passes tokenizeTimeLimit: 0. The grammar is still the real one; only its wall-clock cutoff is off in these tests. The edit is a marked seam in an upstream-owned test, with an UPSTREAM.md note.

Why

highlightNativeReviewDiffVisibleRows > keeps grammar state across inline comment rows dropped #567 from the merge queue. It failed with expected { 'template-open': [ …(3) ] … } to deeply equal { 'template-open': [ …(5) ] … }.

  • Cause: Shiki's default 500 ms per-line limit makes it return partial tokens and incomplete grammar state on a busy runner. The test compares two separately timed calls, so they can disagree.
  • Proof: Codex faked Date.now() so the limit trips mid-test. That reproduced the exact CI diff (3 tokens instead of 5) 5 of 5 times. With this change, it passed 5 of 5.
  • Upstream: the file is upstream-owned and is unchanged through upstream a71a6c37d1, so there was no upstream fix to bring in.

Not changed here: production keeps Shiki's time limit. That means a very slow line can still leave later rows miscoloured in the mobile diff view. Fixing that is a separate decision about responsiveness.

Documentation Impact

Documentation impact: Updated — UPSTREAM.md (test-only override to keep when syncing).

Verification

  • Focused file: 14/14 passed on Windows.
  • Reproduction: the forced-timeout case fails 5/5 without the fix and passes 5/5 with it.
  • Repository checks: format, lint, typecheck, build and git diff --check passed.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI change)
  • I included a video for animation/interaction changes (none)

Root cause and fix: Codex gpt-6.1-sol. Publication: Claude Opus 5.5 in Claude Code (Scient).

🤖 Generated with Claude Code

Shiki 4.2 defaults to a 500ms timeout per line. TextMate can stop partway
through a line, and Shiki publishes those partial tokens and grammar state.
The inline-comment equality test compares separate tokenizations, so runner
contention can make only one hit the cutoff.

Disable that timeout only in the existing real-Shiki test wrapper. Keep
production safeguards and the grammar, batching, and cancellation assertions.
Mark the upstream-owned hunk and document the divergence in UPSTREAM.md.
No changes to these files follow d8e9fd2 through upstream a71a6c3.

Deterministic proof: for the first template tokenization, make Date.now return
0 for seven reads, then 501. The unchanged assertion fails with the exact
three-versus-five-token CI diff in 5/5 runs; with this override it passes 5/5.
The fault injection is not part of the committed test.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS labels Oct 19, 2026
@yaacovcorcos
yaacovcorcos added this pull request to the merge queue Oct 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 26.2 KiB 26.2 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 26.6 KiB 26.6 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: d9f0b39 · PR result: ef117f8 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Full compact HTTP snapshot with live cursor.

  • Codex decoded thread snapshot: 133.1 KiB
  • Claude decoded thread snapshot: 133.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Merged via the queue into main with commit 52586c0 Oct 19, 2026
31 checks passed
@yaacovcorcos
yaacovcorcos deleted the codex/ci-flakes-2-20261020 branch October 19, 2026 23:07
yaacovcorcos pushed a commit to nattikahana/scient-desktop that referenced this pull request Oct 20, 2026
Bring ScientFactory#353 up to current main, including the T3 alignments ScientFactory#593 and ScientFactory#607
(React Compiler migration, browser test project), ScientFactory#596, ScientFactory#601, ScientFactory#603-ScientFactory#606,
ScientFactory#608-ScientFactory#610 and ScientFactory#567.

Composition:
- Workspace writes: ScientFactory#353's mutation service keeps main's known-workspace
  boundary and Git-folder write denial. Every write path (write, create,
  binary write, delete, rename, retained replace/remove, inspect) is
  authorized first, and the mutation lock re-checks protected paths on
  each non-history resolution. Test layers provide KnownWorkspaces.
- Web: conflicted files follow main's React Compiler patterns (no manual
  memo, refs synced in layout effects, render-phase resets, effect
  events, try/finally in module helpers) with ScientFactory#353's behaviour on top,
  including ScientLatexSurface, the PDF reader, the file preview panel,
  save coordinator, workspace refresh, Pandoc tool and source PDF preview.
- Tests moved to main's browser project are ported (createRoot, async
  act); ScientFactory#353's deletions of the Markdown rename button and its seam test
  are kept.
- pnpm-lock.yaml regenerated with pnpm install.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 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.

1 participant