Skip to content

Share one token-link builder across tests (#470) - #499

Merged
alex-rawlings-yyc merged 2 commits into
mainfrom
470-shared-token-link
Oct 23, 2026
Merged

alex-rawlings-yyc merged 2 commits into
mainfrom
470-shared-token-link

Conversation

@alex-rawlings-yyc

@alex-rawlings-yyc alex-rawlings-yyc commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Closes #470.

Summary

The six test files that each built approved token links with their own helper now share makeTokenLink(analysisId, tokenRef, { surfaceText?, status?, id? }) in test-helpers.ts, beside makePhraseLink.

  • Default id JSON-encodes [tokenRef, analysisId, status], so links on one token and payload that differ only in status stay distinct, and colons inside a token ref can't make two ids collide.
  • Status is a builder field. The { ...link, status } spreads in reanchor-analysis.test.ts pass it instead, so the default id tracks it.
  • Id-less links in type-guards.test.ts build through the helper and then delete link.id, as the phrase-link test beside them does.

Evidence

  • Full suite passes with no test outcome changed; lint and tsc clean.

Decisions with a strong alternative

  • Overrides go in an options object. Alternative: a positional surfaceText, mirroring makePhraseLink's surfaceTexts. Why not: overriding only status or id would then need an undefined placeholder.
  • No makeSegmentLink. Only analysis-book.test.ts has a segment-link builder; the other segment links are one-off literals. Alternative: add one for symmetry. Why not: there is nothing to share yet.
  • Scope is the six files in the issue's table. Inline token-link literals elsewhere (analysisSlice.test.ts, AnalysisStore.test.tsx, TokenChip.suggestions.test.tsx) stay as they are. Alternative: migrate them too. Why not: they are literals rather than helpers, and the diff would grow several times over.

Merge Danger

Door: two-way

Tests only. 3d (#443) adds one more local tokenLink in approvalRule.test.ts, which should fold into the shared builder when that branch rebases.

Blast Radius: test-helpers.ts and the six migrated test files

🤖 Generated with Claude Code


This change is Reviewable

Summary by CodeRabbit

  • Tests
    • Standardized token-analysis test data across conformance, validation, summary, and reanchoring coverage. Tests continue to check existing validation outcomes, duplicate IDs, and approved or suggested links; the conformance fixture also covers a low-confidence link. No production behavior or test expectations changed.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 696a7588-1204-44ae-bd4f-c0659b0866f6
📝 Walkthrough

The test helpers add a shared makeTokenLink builder. Six test files replace local or inline token-link fixtures with the shared builder. Existing test scenarios and expectations remain unchanged.

Changes: Token-link test fixtures

Layer / File(s) Summary
Add shared fixture builder
src/__tests__/test-helpers.ts
Adds makeTokenLink with defaults for surface text, status, and ID, plus options to override those values.
Update conformance and type-guard fixtures
src/__tests__/analysis-repository-conformance.ts, src/__tests__/types/type-guards.test.ts
Replaces local or inline token-link fixtures with makeTokenLink. Existing validation expectations remain unchanged.
Update utility-test fixtures
src/__tests__/utils/analysis-book.test.ts, src/__tests__/utils/analysis-violations.test.ts, src/__tests__/utils/project-analysis-summary.test.ts, src/__tests__/utils/reanchor-analysis.test.ts
Replaces local token-link builders with makeTokenLink. Existing test scenarios and assertions remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other

Possibly related PRs

Merge Risk: 🔵 Low · up to d9382

Fix the shared test helper's ID encoding before relying on it for unique fixture IDs.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: replacing duplicated token-link builders with one shared builder across tests.
Linked Issues check Passed Issue #470 is open and directly linked. The diff adds makeTokenLink beside makePhraseLink in src/__tests__/test-helpers.ts. It defaults surfaceText to tokenRef, status to approved, and `…
Out of Scope Changes check Passed The changes stay within issue #470. They modify one shared test helper and the six listed test files. The changes only consolidate test fixtures and preserve the existing assertions. The PR does not m…
Full details: Linked Issues check

Explanation

Issue #470 is open and directly linked. The diff adds makeTokenLink beside makePhraseLink in src/__tests__/test-helpers.ts. It defaults surfaceText to tokenRef, status to approved, and id to ${tokenRef}:${analysisId}:${status}. The helper accepts overrides for all required fields. The six listed test files use the shared helper, including explicit IDs and status overrides. The id-less cases create a helper link and delete its ID. The PR keeps the segment-link builder unchanged because the description reports no duplicated segment-link builder to consolidate. The reported suite, lint, and type-check results support unchanged test behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #470. They modify one shared test helper and the six listed test files. The changes only consolidate test fixtures and preserve the existing assertions. The PR does not modify production code, unrelated test helpers, or the inline token-link literals that the description explicitly excludes.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

coderabbitai[bot]

This comment was marked as resolved.

@imnasnainaec imnasnainaec left a comment

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.

@imnasnainaec reviewed 7 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

@alex-rawlings-yyc
alex-rawlings-yyc merged commit 210b283 into main Oct 23, 2026
10 checks passed
@alex-rawlings-yyc
alex-rawlings-yyc deleted the 470-shared-token-link branch October 23, 2026 18:02
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.

Share one token-link builder across tests

2 participants