Skip to content

chore(fidelity): single upstream pin, SHA check, it.each extraction, two-tier mapping (#185) - #249

Merged
patrick-chinchill merged 11 commits into
mainfrom
sync/4.41-p0
Sep 30, 2026
Merged

patrick-chinchill merged 11 commits into
mainfrom
sync/4.41-p0

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This PR adds the fidelity tooling for the 4.41 wave. The tool can now report what is missing against chat@4.41.1 while CI stays strict at chat@4.31.0. It is tooling only: no runtime changes, no test ports, and the pin does not move (that is #203).

Target report (committed as scripts/fidelity_target.json, chat@4.41.1 @ f690af3c):

TS tests Matched (exact / fuzzy) Missing
Strict tier (MAPPING, 13 files) 836 706 (643 / 63) 130
Target tier (TARGET_MAPPING, 11 files) 200 48 (28 / 20) 152
Total 1036 (incl. 16 .each templates) 754 (671 / 83) 282

The strict-tier missing count by file: chat 33, ai/index 26, thread 18, serialization 17, ai/messages 10, from-full-stream 9, markdown 7, callback-url 5, transcripts-wiring 4, transcripts 1.

The issue estimated about 132; the report says 130. The fuzzy matcher claims one .each template, thread.test.ts's $label template, as the issue predicted. The fuzzy claims are listed per file in fuzzy_matches. The fuzzy pass now matches plain tests before .each templates (see Review), so chat.test.ts's should not treat %s as a mention (%s) template is correctly reported missing, and the plain should not match GitHub-style logins as Slack ids (case sensitivity) test is credited to its existing translation.

Strict at the pin: 733/733. It was 732: thread.test.ts:622's it.each is now counted, and it matches tests/test_thread_faithful.py:953.

What changed

  • Single pin source: scripts/upstream_pin.json. It holds pin (chat@4.31.0 @ 2a553aa9…) and target (chat@4.41.1 @ f690af3c…) with full SHAs.
    • The script reads it; the literals in the old docstring, in the write_baseline fallback and in the clone hint are gone.
    • lint.yml reads it via jq.
    • The CLAUDE.md, docs/UPSTREAM_SYNC.md and CONTRIBUTING.md clone snippets use $(jq -r .pin.tag scripts/upstream_pin.json).
  • SHA pin (closes Pin upstream clone SHA in lint.yml + log it for fidelity reproducibility #79).
    • CI fails if the cloned HEAD differs from pin.sha, and writes the SHA to $GITHUB_STEP_SUMMARY.
    • The script does the same check when TS_ROOT is the top level of a git checkout; --report-target checks against target.sha.
    • A plain export warns without failing. An export nested inside another repo also counts as a plain export.
  • Doc-drift guard. New --check-docs mode, run as a new CI step. It fails if a --branch chat@X or pinned to [vercel/]chat@X phrase in CLAUDE.md or docs/UPSTREAM_SYNC.md differs from pin.tag; the phrase may wrap across lines. This fixed the stale 4.30.0 line. The lint.yml paths filters now include CLAUDE.md, docs/UPSTREAM_SYNC.md and the pin file.
  • Consistency check. The major.minor of pin.tag must equal that of UPSTREAM_PARITY. It runs in every mode, reads __init__.py by regex, and never imports chat_sdk.
  • Extraction.
    • Each it.each / test.each template counts as one logical test. The placeholders %s %d %i %f %j %o %O %c %p %# %$ %%, $name and $a.b are stripped only in .each titles, then ts_name_to_python runs.
    • A bracket-aware scanner finds the title after multi-line tables: ])("…", ] as const)("…", computed )("…" and tagged-template tables. It skips strings, template literals and comments. A dynamic title or a placeholder-only title produces a warning.
    • Plain test("…") now counts; regex.test("…") does not.
    • describe.each titles are used only for reporting.
  • Two-tier mapping.
  • --report-target checks MAPPING ∪ TARGET_MAPPING and never fails on missing tests.
    • It writes the tag, the SHA and totals. Per file, it writes the missing [describe, it] pairs, the extra count, exact/fuzzy counts and the fuzzy pairs.
    • It prints the delta against the committed report.
    • Output is deterministic and puts one pair per line, to keep the parallel wave PRs' diffs small.
  • Exact vs fuzzy counts are printed per file in every mode.
  • Determinism fix (found in self-review). fuzzy_match iterated a set, so score ties were broken by the per-process string hash seed. The target total flipped between 282 and 283 missing across runs. Candidates are now scanned in sorted order. Strict at the pin is 733/733 under 8 different PYTHONHASHSEEDs, and the report is byte-identical across seeds.
  • Docs: docs/UPSTREAM_SYNC.md § Test fidelity was rewritten to cover the pin file, the two tiers, the UNMAPPED reasons table, the target report and the guardrails. CLAUDE.md § Test Quality and CONTRIBUTING.md were updated. CHANGELOG.md gets a new "Unreleased (4.41 wave)" section, marked tooling-only.
  • scripts/fidelity_baseline.json: the total_ts_tests count is 733, and the _comment no longer names the tag.

Upstream mapping

  • 636da193 (#972, chat@4.41.1) pruned and merged tests. The resulting Python "Extra" counts are only reported; each owning wave issue decides what to delete.
  • New core test files come from 169788b6 (history/*), 2ce2be00 (agent-session), 1721fa01 (app-context), 2e2426d1 (installation-events), 2d0d3bf3 (ai/tanstack) and 4cb7e5d5 (workflow/approval). The new .each templates in mapped files come from b547f458, 5b538f6f, 93a58af5, 6f17495b and 056d8830.

Tests

There are no upstream tests to port. New tests/test_verify_test_fidelity.py has 112 tests (44 originally, 44 more from the first review round, 24 more from the gpt-6-astra rounds; see Merge gate) and loads the script via importlib.util.spec_from_file_location. It covers:

  • the real chat@4.41.1 template shapes:
    • should not treat %s as a mention (%s)
    • …via $label
    • retains $expected entries with maxPerUser=$maxPerUser
    • merges legacy maxPerUser=%s under history.user
    • the computed-table $access/$singleton template
    • describe.each(...)("on%s")
  • plain it("100% done") staying unchanged, and regex.test not counting;
  • the placeholder sweep, including %%s, and the scanner ignoring brackets inside strings and comments;
  • SHA mismatch failing and a match passing, a plain export only warning, and checkout top-level detection on a real temporary git repo;
  • doc drift failing on a stale --branch chat@4.30.0 and on a wrapped pinned\nto;
  • completeness flagging an unlisted *.test.ts;
  • the pin/UPSTREAM_PARITY major.minor check;
  • the report's content and JSON round-trip, deterministic fuzzy tie-breaks, and mode exclusivity.

I mutation-checked the tests against 11 injected faults and 10 were caught. The one survivor is an equivalent mutant: reordering the %% alternative, where % is not in the character class. That first round did not cover the CLI wiring. In the review round I ran a second check of 17 faults, covering the reviewer's 7 survivors plus faults in each new guard, and all 17 were caught.

Divergences

None. This is tooling only and adds no non-parity rows.

Consumer impact

None; this is tooling only.

Validation

ruff check, ruff format --check, audit_test_quality.py (0 hard failures), --check-docs, strict fidelity at chat@4.31.0 (733/733), pytest (5451 passed, 13 skipped, after merging main) and pyrefly (0 errors) all pass.

One note on the environment (resolved: main now caps the Teams SDK below 2.1, #251, and this branch has merged it): uv.lock is gitignored. A fresh uv sync resolves microsoft-teams-apps 2.1.0, and with it 3 existing Teams tests fail with 'App' object has no attribute 'activity_sender'. They pass with 2.0.13.4, which is what the main checkout's venv uses. This is unrelated to this PR and does not touch src/ or the Teams tests. CI resolves fresh, so it may hit the same failure.

Follow-ups

Review

Two reviewers sent 11 findings. I checked each one myself: 10 are fixed in 958c8f2 and 1 is declined. Every fix has a test that fails without it.

Fixed

  1. (major) An .each template could steal a plain test's fuzzy match.

    • check_fidelity pass 2 now fuzzy-matches plain tests first, then .each templates; the report stays in file order.
    • I regenerated fidelity_target.json. chat.test.ts "should not match GitHub-style logins as Slack ids (case sensitivity)" is now matched, and should not treat %s as a mention (%s) is reported missing. The thread $label template still matches, and the total stays at 282.
    • Tests: test_fuzzy_pass_matches_plain_tests_before_each_templates and test_committed_target_report_credits_the_plain_github_login_test.
  2. (minor) Vitest .each variants were dropped with no warning.

    • The extractor now reads:
      • modifier chains (skip, only, concurrent, sequential, fails, shuffle) before .each / .for;
      • <T> type arguments, including nested generics and =>;
      • .skipIf(c) / .runIf(c);
      • plain modifiers such as it.skip("…");
      • single-quoted titles, and titles that start on the next line.
    • Any other it/test/describe call form (it.todo, it.extend, a non-literal title) produces a warning.
    • Extraction over both upstream checkouts is byte-identical to before.
    • Tests: test_vitest_call_variants_are_extracted, test_unknown_test_call_forms_warn_instead_of_vanishing and test_prose_and_strings_are_not_test_calls.
  3. (minor) --report-target labelled every miss "MISSING (baselined)". It now prints plain MISSING:. Test: test_report_target_lists_missing_without_a_baseline_marker.

  4. (minor) The SHA-mismatch hint re-cloned into the existing TS_ROOT.

    • It now suggests a fresh sibling directory named for the tag, or points to that directory if it already exists.
    • Test: test_sha_mismatch_hint_clones_into_a_fresh_directory.
  5. (minor) The Extend verify_test_fidelity.py MAPPING to cover all packages/chat/src/*.test.ts #78 progress comment had not been posted. It is posted now; see Follow-ups.

  6. (major) The CI-facing wiring was untested. New CLI-level tests cover:

    • --check-docs returns 0 for the pin, and 1 for a stale tag or the target tag;
    • --report-target passes with the target SHA, and fails with the pin SHA or any other SHA, without writing the report;
    • --report-target fails on a MAPPING/TARGET_MAPPING overlap;
    • the total and per-file delta output;
    • --strict fails on a pin/UPSTREAM_PARITY mismatch.

    All 7 of the reviewer's surviving mutants are now caught (17 of 17 in my re-run).

  7. (major) --strict passed when a .each template could not be extracted.

    • --strict and --report-target now treat every extraction warning as an error: they list file:line, exit 1, and --report-target does not write the report. Baseline mode still only warns.
    • Tests use the reviewer's three shapes (a regex literal with an unbalanced bracket, a ${} title, a placeholder-only title): test_strict_fails_closed_on_unextractable_upstream_tests, test_report_target_fails_closed_… and test_baseline_mode_only_warns_….
  8. (minor) The SHA check passed when git failed on a real checkout.

    • If TS_ROOT/.git exists but git cannot read HEAD, the check now fails with git's stderr. The warning is kept only for true plain exports.
    • Test: test_sha_check_fails_closed_when_git_cannot_read_an_existing_dot_git.
  9. (minor) A checkout with a pruned working tree passed as "verified".

    • After HEAD matches, git status --porcelain --untracked-files=no -- packages/chat/src must be empty.
    • Test: test_sha_check_rejects_local_edits_to_upstream_test_files.
  10. (minor) The --check-docs regex missed common variants.

    • It is now case-insensitive and accepts --branch=, -b, a \ line continuation, pinned to the …, and ** / _ / backtick / [ decoration.
    • All 8 missed probes plus the 2 already caught are tests (test_doc_drift_catches_stale_pin_variants). The real docs still pass.
  11. (minor) The docs contradicted the SHA check.

    • In docs/UPSTREAM_SYNC.md, the full clone for reading diffs (step 1 and "How to Diff Upstream Changes") now lives at /tmp/vercel-chat-full. /tmp/vercel-chat is always the pinned shallow checkout, as in step 5, the "Final parity check" row, CLAUDE.md and CI.
    • The guardrails section now documents the new fail-closed checks.

Declined: none. Finding 5 is a process item rather than code; I resolved it by posting the comment.

CI note, outside this PR (resolved by #251, merged into this branch; CI is now green): test (3.12) / test (3.13) fail on 3 Teams tests ('App' object has no attribute 'activity_sender', and fake_update() got an unexpected keyword argument 'service_url'). CI resolves microsoft-teams-apps 2.1.0 fresh, because uv.lock is gitignored. The same tests fail on main's code, since this PR touches no src/ or Teams tests. With the SDK overlaid at 2.0.13.4 the whole suite passes: 5235 passed, 13 skipped. Lint & Type Check is skipped because the PR is a draft (if: !github.event.pull_request.draft). I ran every lint step locally, and all pass: ruff, format, audit (0 hard failures), --check-docs, strict 733/733 at chat@4.31.0, and pyrefly (0 errors).

Closes #185
Closes #79
Part of #184

Merge gate

Final HEAD: 691d815 (includes main @ c77730b).

gpt-6-astra: 6 runs. Final verdict on 691d815: "No actionable regressions found. All 112 focused tests passed, as did lint, formatting, doc-pin validation, and strict fidelity verification at 733/733. Regenerating the target report reproduced the committed report exactly."

Round HEAD Verdict
1 50be6a2 (after merging main / #251) 2 × P2 (fixed in 89fb863)
2 89fb863 2 × P2 (fixed in ea94072)
3 ea94072 clean
4 d1e500c (after merging main again) 2 × P2 (fixed in 5e528c0)
5 5e528c0 clean
6 691d815 (after merging main again) clean

Fixed. Each fix has a test that fails on the previous commit. After every fix, extraction over every upstream packages/**/*.test.ts(x) at both chat@4.31.0 and chat@4.41.1 (9,516 lines of output) was byte-identical to before the fix. Strict stays 733/733, and the target report stays 282 missing.

  1. A describe modifier used up the line's test slot. describe.only("s", () => { it("t") }) dropped it("t"). Scanning now continues after a describe call. Test: test_describe_call_does_not_consume_the_line_test (describe.only, .skip, .each).
  2. Test calls inside strings and comments were discovered. A 'test("x")' fixture string counted as a test, and a commented-out // it.each(cases)(name, fn) was reported as unextractable, which aborted --strict. A new code_mask lexer marks comments and string, template and regex literals, and calls are only discovered in code. Tests: test_calls_inside_strings_comments_and_regexes_are_ignored (8 shapes) and test_real_tests_around_literals_are_still_extracted.
  3. A test pinned the live report's gaps. test_committed_target_report_credits_the_plain_github_login_test asserted that the committed fidelity_target.json still lists a missing test, so a wave PR that ports it would have failed. It is removed. The matcher regression stays covered by the synthetic test_fuzzy_pass_matches_plain_tests_before_each_templates.
  4. The --report-target delta compared against the working copy. A second run before committing compared against the first run's output and printed +0. The delta now reads the report committed at HEAD via git show. Tests: test_report_target_prints_delta_against_the_report_committed_at_head (two runs, both -1) and test_report_target_ignores_an_uncommitted_working_copy_for_the_delta.
  5. A regex right after if (…) was lexed as division. In if (x) //.test(v), the regex's backtick opened a template that hid the following tests. The lexer now tracks which (followsif/while/for/with. As defense in depth, a template literal, ${…}or block comment still open at end of file is an extraction error, so it fails--strict; the file then falls back to scanning everything as code. Tests: test_regex_after_control_flow_parens_does_not_mask_later_testsandtest_unterminated_literal_fails_closed_instead_of_masking_the_rest`.
  6. A computed title was matched by its literal prefix. it.each(…)("rejects " + kind + " %s") was read as "rejects ". A title literal must now be followed by , or ), and otherwise it is an extraction error. This applies to plain, .each and .skipIf calls. Tests: test_computed_title_is_unextractable_not_its_literal_prefix and test_literal_title_followed_by_whitespace_or_paren_still_extracted.

Rebutted: none.

Bot comments: CodeRabbit only posted its rate-limit notice ("Review limit reached") and never reviewed. gemini-code-assist did not post. No inline comments or reviews exist, so there was nothing to address.

CI on 691d815: all green. Lint & Type Check (every step, including the pin SHA check, --check-docs, strict fidelity and pyrefly), test (3.12), test (3.13), CodeQL (Analyze (actions), Analyze (python)).

Local validation on 691d815: ruff check, ruff format --check, audit_test_quality.py (0 hard failures), --check-docs, strict fidelity at chat@4.31.0 (733/733), pytest (5451 passed, 13 skipped), pyrefly (0 errors). --report-target at chat@4.41.1 reproduces the committed report (282 → 282).

…two-tier mapping (#185)

- scripts/upstream_pin.json is the single pin source (pin chat@4.31.0 and
  target chat@4.41.1, with full commit SHAs); the script, lint.yml (jq) and
  the CLAUDE.md clone snippet read it.
- CI and the script fail when the checkout HEAD differs from the pinned SHA
  (closes #79); the script only warns for a plain export.
- --check-docs guards CLAUDE.md / docs/UPSTREAM_SYNC.md against stale pin
  literals; pin major.minor must match UPSTREAM_PARITY.
- it.each / test.each templates count as one logical test (placeholders
  stripped); test() counts; describe.each titles are used for reporting.
  Strict fidelity at the pin is now 733/733.
- Two tiers: MAPPING (strict) + TARGET_MAPPING (--report-target only) +
  UNMAPPED (reasons), with a completeness check over core *.test.ts(x).
- --report-target writes scripts/fidelity_target.json (committed for
  chat@4.41.1: 282 missing of 1036, incl. 16 .each templates).
- Fuzzy-match ties now break deterministically (sorted candidates).

Tooling only; no consumer-visible behavior change. Part of #184.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f226cb24-e392-42e1-ae35-d7253d368bca

📥 Commits

Reviewing files that changed from the base of the PR and between 6700926 and 209230e.

📒 Files selected for processing (10)
  • .github/workflows/lint.yml
  • CHANGELOG.md
  • CLAUDE.md
  • CONTRIBUTING.md
  • docs/UPSTREAM_SYNC.md
  • scripts/fidelity_baseline.json
  • scripts/fidelity_target.json
  • scripts/upstream_pin.json
  • scripts/verify_test_fidelity.py
  • tests/test_verify_test_fidelity.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

…uzzy order, checkout/doc guards (#185)

- Fuzzy pass matches plain tests before .each templates; regenerated
  fidelity_target.json (chat.test.ts GitHub-login test now credited, the
  isMention template reported missing; total stays 282).
- Extractor reads modifier chains, .for, <T> type args, skipIf/runIf,
  single-quoted and wrapped titles; unknown call forms warn.
- --strict / --report-target fail on any extraction warning.
- SHA check fails when TS_ROOT/.git exists but git cannot read it, and
  on local edits under packages/chat/src; mismatch hint clones into a
  fresh directory.
- --check-docs: case-insensitive, --branch=, -b, continuations, markdown.
- --report-target prints plain MISSING (no baseline marker).
- UPSTREAM_SYNC.md: full clone at /tmp/vercel-chat-full, pinned checkout
  at /tmp/vercel-chat.
- CLI-level tests for --check-docs, --report-target SHA/overlap/delta and
  the parity check.
@patrick-chinchill
patrick-chinchill marked this pull request as ready for review September 30, 2026 06:40
@patrick-chinchill

Copy link
Copy Markdown
Collaborator Author

Merge gate: CI green (Lint & Type Check, test (3.12), test (3.13), CodeQL, Analyze (actions), Analyze (python)); local Codex review (gpt-6-astra, xhigh, --base origin/main) on 209230e: "No actionable regressions found. All 116 tooling tests passed, strict fidelity verified 733/733 tests against the pinned upstream commit, and regenerating the target report reproduced the committed report exactly."; 2 astra rounds after merging main (#205, #231, #243): round 1 found a P2 (--report-target silently recorded mapped files as absent from a sparse/partial checkout or plain export), fixed in 209230e; round 2 clean. Earlier convergence was on 691d815. Bot reviews: CodeRabbit rate-limited (status pass), no other bot or human reviews. Merging with --admin (Protect Main requires a code-owner approval).

@patrick-chinchill
patrick-chinchill merged commit 4817dc0 into main Sep 30, 2026
7 checks passed
@patrick-chinchill
patrick-chinchill deleted the sync/4.41-p0 branch September 30, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant