feat: add expected patterns, pnpm migration, docs & CI overhaul#151
feat: add expected patterns, pnpm migration, docs & CI overhaul#151
Conversation
The greedy .+ pattern captured the entire text2 string, so there were no insert segments to assert. Constraining to \w+ captures only "Jason", leaving "coding" vs "hacking" as a real deviation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
svelte-eslint-parser does not implement scopeManager.addGlobals() required by ESLint 10. Only affects .svelte.ts runes modules; regular .ts and .svelte files lint fine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Found 5 test failures on Blacksmith runners: Failures
|
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughUpdates include: (1) .trunk/trunk.yaml adds ESLint to lint.ignore for files matching **/*.svelte.ts. (2) tests/expected-patterns.test.ts tightens a capture from Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Runs on pull requests to main when src/tests change. Unit tests run first, then sharded Playwright e2e tests across 2 shards. Uses pnpm, caches build/vitest/playwright artifacts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/run-tests.yml:
- Around line 12-22: The workflow's path filters under the paths: list are
missing pnpm-lock.yaml so changes to the pnpm lockfile won't trigger the job;
update the paths: entries to include "pnpm-lock.yaml" alongside package.json and
package-lock.json so lockfile-only changes kick off the workflow (modify the
paths: section in the run-tests.yml workflow to add pnpm-lock.yaml).
- Around line 115-136: The workflow is missing the build step required by the
Playwright webServer in playwright.config.ts; add a step that runs "pnpm build"
(before the "Run e2e tests (sharded)" step) so the dist directory exists for
tests, and ensure the webServer command in playwright.config.ts is consistent
with the package manager by changing its startup command from "npm run build &&
npm run preview" to "pnpm build && pnpm preview" (or equivalent) so builds and
previews use the same package manager.
| paths: | ||
| - src/** | ||
| - tests/** | ||
| - '!docs/**' | ||
| - package.json | ||
| - package-lock.json | ||
| - svelte.config.* | ||
| - vite.config.* | ||
| - playwright.config.* | ||
| - tsconfig*.json | ||
| - .github/workflows/run-tests.yml |
There was a problem hiding this comment.
Add pnpm-lock.yaml to the workflow path filters.
Dependency-only changes to the pnpm lockfile won’t trigger this workflow, so tests can be skipped.
🔧 Proposed fix
paths:
- src/**
- tests/**
- '!docs/**'
- package.json
+ - pnpm-lock.yaml
- package-lock.json
- svelte.config.*
- vite.config.*
- playwright.config.*
- tsconfig*.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| paths: | |
| - src/** | |
| - tests/** | |
| - '!docs/**' | |
| - package.json | |
| - package-lock.json | |
| - svelte.config.* | |
| - vite.config.* | |
| - playwright.config.* | |
| - tsconfig*.json | |
| - .github/workflows/run-tests.yml | |
| paths: | |
| - src/** | |
| - tests/** | |
| - '!docs/**' | |
| - package.json | |
| - pnpm-lock.yaml | |
| - package-lock.json | |
| - svelte.config.* | |
| - vite.config.* | |
| - playwright.config.* | |
| - tsconfig*.json | |
| - .github/workflows/run-tests.yml |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/run-tests.yml around lines 12 - 22, The workflow's path
filters under the paths: list are missing pnpm-lock.yaml so changes to the pnpm
lockfile won't trigger the job; update the paths: entries to include
"pnpm-lock.yaml" alongside package.json and package-lock.json so lockfile-only
changes kick off the workflow (modify the paths: section in the run-tests.yml
workflow to add pnpm-lock.yaml).
| - name: Install dependencies | ||
| run: pnpm install --frozen-lockfile | ||
|
|
||
| - name: Cache Playwright browsers | ||
| uses: actions/cache@v5 # zizmor: ignore[unpinned-uses] | ||
| with: | ||
| path: ~/.cache/ms-playwright | ||
| key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-playwright- | ||
|
|
||
| - name: Install Playwright browsers | ||
| run: pnpm exec playwright install --with-deps | ||
|
|
||
| - name: Sanity check - list discovered tests | ||
| run: pnpm exec playwright test --list | head -n 50 | cat | ||
|
|
||
| - name: Run e2e tests (sharded) | ||
| env: | ||
| CI: true | ||
| run: pnpm exec playwright test --config=playwright.config.ts --shard=${{ matrix.shard }} --reporter=line | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "playwright.config.ts" -o -name "playwright.config.js" | head -5Repository: humanspeak/svelte-diff-match-patch
Length of output: 100
🏁 Script executed:
find . -name "package.json" | head -3Repository: humanspeak/svelte-diff-match-patch
Length of output: 112
🏁 Script executed:
cat -n playwright.config.tsRepository: humanspeak/svelte-diff-match-patch
Length of output: 1395
🏁 Script executed:
cat -n package.jsonRepository: humanspeak/svelte-diff-match-patch
Length of output: 5017
Build step is required and missing.
The playwright.config.ts webServer runs npm run build && npm run preview on startup. Without a build step in the workflow, the dist directory won't exist and tests will fail. Add pnpm build before the e2e tests.
Additionally, playwright.config.ts uses npm run but the workflow uses pnpm—consider updating the webServer command to pnpm build && pnpm preview for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/run-tests.yml around lines 115 - 136, The workflow is
missing the build step required by the Playwright webServer in
playwright.config.ts; add a step that runs "pnpm build" (before the "Run e2e
tests (sharded)" step) so the dist directory exists for tests, and ensure the
webServer command in playwright.config.ts is consistent with the package manager
by changing its startup command from "npm run build && npm run preview" to "pnpm
build && pnpm preview" (or equivalent) so builds and previews use the same
package manager.
Replace page.fill('[data-testid=...]') with page.getByTestId().fill()
to fix webkit race condition where CSS selector fills didn't update
Svelte reactive state before assertions fired.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
(?<name>pattern)inoriginalTextto mark dynamic regions (dates, names, versions) with distinct "expected" styling instead of insert/remove colorsChanges
expectedsnippet renderer andrendererClasses.expectedCSS class supportCaptureRange,DisplayDiff,PatternMatchResulttypescapturesas 3rd argument toonProcessingcallbackextractCaptures,tagExpectedRegions,cleanTemplate.svelte.tsfiles (ESLint 10 / svelte-eslint-parser incompatibility)Commits
bf85395chore(trunk): ignore eslint on .svelte.ts files39724c2fix(test): use \w+ instead of .+ in e2e capture patternc932be5chore: tune coderabbit config for TypeScript/Svelte project65116e2docs: add comprehensive JSDoc for all public types, interfaces, and componentcc94ec8docs: update CLAUDE.md to use pnpm commands80f5f02ci: rename trunk-check workflow and add security hardeningd5706c7ci: remove unused www and workers install steps from setup-ci6dfe16ddocs: add expected patterns section to READMEb57ca4ffix(expected-patterns): replace anchored regex with gap-flexible matching2f03924build: migrate to pnpm and update all dependenciesTest plan
npx vitest run src/lib/— 41 unit tests passnpx playwright test tests/expected-patterns.test.ts— 25 e2e tests pass (5 browsers × 5 tests)trunk checkpasses on all source files🤖 Generated with Claude Code