Skip to content

feat(web): add whitespace filters to PR diffs - #53

Merged
kalvenschraut merged 1 commit into
rtvisionfrom
feat/pr-review-whitespace
Sep 12, 2026
Merged

kalvenschraut merged 1 commit into
rtvisionfrom
feat/pr-review-whitespace

Conversation

@kalvenschraut

Copy link
Copy Markdown
Member

Adding a wrapping element can mark every indented child as changed in a PR diff. The Code tab now offers whitespace comparisons for all changes, ignored whitespace, ignored spacing amounts, and ignored end-of-line whitespace, for both PRs and individual commits.

Filtering runs in a worker. Multi-hunk patches use validated full revisions where the host supports loading them. Review positions still resolve against the original patch, existing filtered files stay mounted during pagination, and pending comments on hidden lines remain available above the diff.

Validation: 30 focused tests, web typecheck, targeted lint, and formatting checks pass. The production worker was previously built and exercised against PR pingdotgg#2001; its template reduces to 12 additions and zero deletions, matching the requested comparison. Fable approved the implementation after its review findings were fixed. The maintainer explicitly waived browser testing and PR images.

Implemented with GPT-6 through Codex. Independent review by Claude Fable 5.1 through Claude Code.

@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Sep 12, 2026
@kalvenschraut

Copy link
Copy Markdown
Member Author

@coderabbitai review

Please review the current head, 58f02511a339733f6292269394ad6aef30ef80dc, and submit an APPROVED review if no blocking findings remain. The maintainer has authorized merging only after your approval and completed CI. Focus on whitespace semantics, original comment positions, pagination and worker races, and visibility of pending comments on hidden lines. Browser testing and PR images were explicitly waived by the maintainer.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Current commit =='
git rev-parse HEAD
git status --short

echo '== PR metadata and checks =='
gh pr view 53 --repo RTVision/t3code --json number,title,baseRefName,headRefName,headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup,files --jq '{
  number,
  title,
  baseRefName,
  headRefName,
  headRefOid,
  mergeStateStatus,
  reviewDecision,
  checks: [.statusCheckRollup[]? | {name, status, conclusion}]
}'

echo '== Changed files =='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD || true

echo '== Diff stat =='
git diff --stat "$(git merge-base HEAD origin/main)" HEAD || true

echo '== Available source/test files related to diff views and whitespace =='
fd -t f . | rg -i '(diff|patch|whitespace|worker|comment|review|pagination)' | head -250

Length of output: 48357


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="$(git merge-base HEAD origin/rtvision)"
echo "== Effective review range =="
echo "$base..HEAD"
git diff --name-status "$base" HEAD
git diff --stat "$base" HEAD

echo '== Changed whitespace implementation files =='
git diff --name-only "$base" HEAD -- 'apps/web/src/components/pullRequest/*' |
  rg '(PullRequestCodeTab|pullRequestWhitespace|usePullRequestWhitespace)' || true

echo '== Source outlines =='
for file in \
  apps/web/src/components/pullRequest/PullRequestCodeTab.tsx \
  apps/web/src/components/pullRequest/pullRequestWhitespace.ts \
  apps/web/src/components/pullRequest/pullRequestWhitespace.worker.ts \
  apps/web/src/components/pullRequest/usePullRequestWhitespace.ts \
  apps/web/src/components/pullRequest/pullRequestWhitespace.test.ts \
  apps/web/src/components/pullRequest/usePullRequestWhitespace.test.tsx
do
  if test -f "$file"; then
    echo "--- $file ---"
    ast-grep outline "$file" --items all || true
  fi
done

echo '== Targeted diff =='
git diff --find-renames --unified=80 "$base" HEAD -- \
  apps/web/src/components/pullRequest/PullRequestCodeTab.tsx \
  apps/web/src/components/pullRequest/pullRequestWhitespace.ts \
  apps/web/src/components/pullRequest/pullRequestWhitespace.worker.ts \
  apps/web/src/components/pullRequest/usePullRequestWhitespace.ts \
  apps/web/src/components/pullRequest/pullRequestWhitespace.test.ts \
  apps/web/src/components/pullRequest/usePullRequestWhitespace.test.tsx

Length of output: 1831


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== PR revision range =='
gh pr view 53 --repo RTVision/t3code --json baseRefOid,headRefOid,commits --jq '{
  baseRefOid,
  headRefOid,
  commits: [.commits[] | {oid, messageHeadline}]
}'

echo '== PR changed files matching whitespace feature =='
gh api --paginate 'repos/RTVision/t3code/pulls/53/files?per_page=100' \
  --jq '.[] | select(.filename | test("apps/web/src/components/pullRequest/(PullRequestCodeTab|pullRequestWhitespace|usePullRequestWhitespace)")) | {filename,status,additions,deletions,patch}'

echo '== Local refs that can establish the review base =='
git show-ref --heads --remotes | rg 'rtvision|58f02511' || true
git branch -a --contains HEAD || true

Length of output: 42983


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Whitespace module with line numbers =='
cat -n apps/web/src/components/pullRequest/pullRequestWhitespace.ts
echo '== Whitespace hook with line numbers =='
cat -n apps/web/src/components/pullRequest/usePullRequestWhitespace.ts

echo '== Code-tab relevant regions =='
sed -n '470,660p;720,780p;900,950p;1450,1590p' apps/web/src/components/pullRequest/PullRequestCodeTab.tsx | cat -n

echo '== Hunk content consumers and FileDiff shape usage =='
rg -n -C 4 '\.hunkContent\b|hunkContent:' apps/web/src packages --glob '*.{ts,tsx}' | head -300

echo '== Review position and pending-comment helpers =='
rg -n -C 5 'function (resolveDiffReviewPosition|getReviewPositionAnchor|isLineInFileDiff|resolveFileDiffPath)|const (resolveDiffReviewPosition|getReviewPositionAnchor|isLineInFileDiff|resolveFileDiffPath)' \
  apps/web/src --glob '*.{ts,tsx}'

echo '== Diff content loader contract =='
rg -n -C 6 'createPullRequestDiffFileContentsLoader|diffFileContents' \
  apps/web/src packages/client-runtime/src --glob '*.{ts,tsx}' | head -300

Length of output: 53162


@kalvenschraut I found no blocking issue in the inspected whitespace-filter implementation.

The code preserves original diff coordinates for new review comments. It keeps filtered files mounted during pagination. It cancels obsolete worker work. It shows pending comments that filtering hides in the comments section above the diff.

I did not submit approval. Required CI checks are still in progress, and the PR merge state is UNSTABLE. After the required checks succeed, post a top-level @coderabbitai approve command to request the approval.

You are interacting with an AI system.

@kalvenschraut
kalvenschraut changed the base branch from rtvision to fix/release-smoke-locked-dependencies September 12, 2026 16:35
@kalvenschraut
kalvenschraut force-pushed the feat/pr-review-whitespace branch from 58f0251 to de0233b Compare September 12, 2026 16:35
kalvenschraut added a commit that referenced this pull request Sep 12, 2026
Release Smoke started failing on unrelated PRs when fresh resolution selected an `expo-audio` version that does not match the repository's version-specific patch. The smoke test deleted its copied lockfile before installing; the release workflow keeps that lockfile.

Keep the copied lockfile when checking the release version bump, so the smoke test uses the same locked dependency versions as a release. This unblocks CI for #52 and #53 without changing application dependencies.

Validation: the complete release-smoke script passes locally with `CI=true`; targeted lint and formatting checks pass. The failing hosted jobs report `ERR_PNPM_UNUSED_PATCH` for `expo-audio@57.0.4` before this change.

Implemented with GPT-6 through Codex.
@kalvenschraut
kalvenschraut changed the base branch from fix/release-smoke-locked-dependencies to rtvision September 12, 2026 16:42
@github-actions

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.6 KiB +29 B (+0.2%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.0 KiB −5 B (−0.1%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.6 KiB +34 B (+0.5%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 57.0 KiB 57.1 KiB +44 B (+0.1%) 66.4 KiB ✅
Codex Live turn messages 9 10 +1 (+11.1%) 21 ✅
Claude Total thread wire 13.6 KiB 13.6 KiB +21 B (+0.2%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.5 KiB 6.6 KiB +19 B (+0.3%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.8 KiB 57.9 KiB +44 B (+0.1%) 66.4 KiB ✅
Claude Live turn messages 9 10 +1 (+11.1%) 21 ✅

Baseline: 1a491d0 · PR result: de0233b · 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.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

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

@kalvenschraut

Copy link
Copy Markdown
Member Author

@coderabbitai approve

All checks now pass on current head de0233bf65a58bc20651864d6e8bb35bfde94170, including Check, Test, all three server-test jobs, Rust, and Release Smoke. The release-smoke fix was reviewed and merged separately in #54. This PR targets rtvision again.

You found no blocking issue in the whitespace implementation at 58f02511a339733f6292269394ad6aef30ef80dc. The only change since then was rebasing that identical feature commit onto the CI fix; git range-diff confirms the feature diff is unchanged. Please confirm approval of the current head and submit an APPROVED review.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="RTVision/t3code"
expected_head="de0233bf65a58bc20651864d6e8bb35bfde94170"
previous_head="58f02511a339733f6292269394ad6aef30ef80dc"

echo "== Pull request state =="
gh pr view 53 --repo "$repo" --json number,baseRefName,headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup --jq '{
  number,
  baseRefName,
  headRefOid,
  mergeStateStatus,
  reviewDecision,
  checks: [.statusCheckRollup[]? | {name, status, conclusion}]
}'

echo "== Head verification =="
actual_head="$(gh pr view 53 --repo "$repo" --json headRefOid --jq .headRefOid)"
printf 'expected=%s\nactual=%s\n' "$expected_head" "$actual_head"
test "$actual_head" = "$expected_head"

echo "== Commit relationship and range comparison =="
git cat-file -e "${expected_head}^{commit}"
git cat-file -e "${previous_head}^{commit}"
git merge-base --is-ancestor "$previous_head" "$expected_head" && echo "previous head is an ancestor" || echo "previous head was rewritten"

base="$(git merge-base "$previous_head" "origin/rtvision")"
new_base="$(git merge-base "$expected_head" "origin/rtvision")"
printf 'previous-base=%s\ncurrent-base=%s\n' "$base" "$new_base"
git range-diff --no-color "$base..$previous_head" "$new_base..$expected_head" || true

echo "== Current PR diff files =="
git diff --name-status "$new_base" "$expected_head"

echo "== Required check status summary =="
gh pr checks 53 --repo "$repo" --required || true

Length of output: 3589


🏁 Script executed:

#!/bin/bash
set -euo pipefail

old="58f02511a339733f6292269394ad6aef30ef80dc"
new="de0233bf65a58bc20651864d6e8bb35bfde94170"

echo "== Commit metadata =="
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$old"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$new"

old_parent="$(git show -s --format=%P "$old" | awk '{print $1}')"
new_parent="$(git show -s --format=%P "$new" | awk '{print $1}')"
printf 'old_parent=%s\nnew_parent=%s\n' "$old_parent" "$new_parent"

echo "== Feature patch identity =="
git diff --no-ext-diff --binary "$old_parent" "$old" > /tmp/old-feature.patch
git diff --no-ext-diff --binary "$new_parent" "$new" > /tmp/new-feature.patch
printf 'old_patch_sha256='
sha256sum /tmp/old-feature.patch | awk '{print $1}'
printf 'new_patch_sha256='
sha256sum /tmp/new-feature.patch | awk '{print $1}'
git diff --no-index --stat /tmp/old-feature.patch /tmp/new-feature.patch || true
git diff --no-index --exit-code /tmp/old-feature.patch /tmp/new-feature.patch >/dev/null && echo "feature patches are identical"

echo "== Current PR required checks =="
gh pr checks 53 --repo RTVision/t3code --required

echo "== Current PR review and merge state =="
gh pr view 53 --repo RTVision/t3code --json headRefOid,baseRefName,mergeStateStatus,reviewDecision --jq .

Length of output: 2352


@kalvenschraut Approved PR #53 at de0233bf65a58bc20651864d6e8bb35bfde94170.

The rebased feature patch is byte-for-byte identical to the reviewed patch. The PR is clean, and the reported checks have passed.

✅ Action performed

Comments resolved and changes approved.

@kalvenschraut
kalvenschraut merged commit 77645c9 into rtvision Sep 12, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 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