Skip to content

ci(frontend): make checks deterministic - #71

Merged
rmens merged 6 commits into
mainfrom
ci/deterministic-frontend-checks
Jul 30, 2026
Merged

ci(frontend): make checks deterministic#71
rmens merged 6 commits into
mainfrom
ci/deterministic-frontend-checks

Conversation

@rmens

@rmens rmens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pin TypeScript 7.0.2 and the Bun/Node type packages in package.json and bun.lock
  • expose one local check:frontend command for Biome, TypeScript and Bun unit tests, and run that exact command in CI
  • expand Biome and TypeScript coverage to maintained source, unit-test, e2e and Playwright files while keeping generated assets out of scope
  • migrate the Biome 2.5 configuration and format the newly covered baseline files
  • include JS unit tests in frontend CI

Verification

  • bun install --frozen-lockfile
  • bun run check:frontend — 49 files checked, TypeScript clean, 5 tests passed
  • bun run build
  • vendor/bin/phpunit — 377 tests, 696 assertions
  • vendor/bin/phpstan analyse --memory-limit=1G --debug
  • vendor/bin/phpcs
  • composer validate --strict

Closes #61

Summary by CodeRabbit

  • Chores
    • Updated CI to trigger on workflow changes and broaden lint coverage; consolidated frontend validation into a single check (JS lint, TypeScript typecheck, and JS tests) and refreshed the lint workflow job.
    • Pinned the Bun runtime and reworked validation scripts (check:frontend), plus adjusted release/package exclusions to omit all tsconfig*.json.
    • Added tsconfig.tests.json for test-oriented TypeScript typing.
  • Documentation
    • Updated README command table for the new lint/typecheck/check flows.
  • Style
    • Reformatted CSS and end-to-end specs; updated lint configuration and minor typing/lint suppression details.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@rmens, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 55 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 256d05b1-c1df-494f-88a8-d41d16120cc2

📥 Commits

Reviewing files that changed from the base of the PR and between 49a833b and 2e47424.

📒 Files selected for processing (2)
  • README.md
  • tests/e2e/helpers.ts
📝 Walkthrough

Walkthrough

Frontend linting, typechecking, and Bun tests are consolidated into shared local and CI commands. Biome and TypeScript coverage expands to source, CSS, tests, and Playwright configuration, with related formatting and compatibility updates.

Changes

Frontend checks

Layer / File(s) Summary
Frontend tooling scope
biome.json, tsconfig.json, tsconfig.tests.json, package.json
Biome and TypeScript now cover maintained frontend, test, CSS, and Playwright files, with pinned Bun, Biome, and TypeScript tooling.
Local check commands
package.json, README.md
Shared PHP/frontend check scripts, local typechecking, expanded Biome targets, and their documentation are updated.
CI execution and release packaging
.github/workflows/js-lint.yml, .github/workflows/release.yml, bin/package-plugin.sh
CI uses the shared frontend check command and updated path filters; release packaging excludes all tsconfig*.json files.
Lint-driven source and test updates
resources/css/*, resources/ts/*, tests/e2e/*
CSS, JavaScript, TypeScript, and Playwright files receive formatting, import-order, compatibility, and typing updates without described behavior changes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant BunScripts
  participant FrontendChecks
  GitHubActions->>BunScripts: run check:frontend
  BunScripts->>FrontendChecks: run lint:js, typecheck, and test:js
  FrontendChecks-->>GitHubActions: report check results
Loading

Possibly related PRs

Suggested labels: dependencies, javascript

Suggested reviewers: mitchellklijs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 is concise and matches the PR’s main change: making frontend CI checks deterministic.
Linked Issues check ✅ Passed The PR pins TypeScript, adds check:frontend, expands Biome/TypeScript coverage, and runs bun run test:js in CI as requested.
Out of Scope Changes check ✅ Passed The changes are all related to frontend CI, linting, typing, or supporting docs/config; no unrelated codepaths stand out.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch ci/deterministic-frontend-checks
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/deterministic-frontend-checks

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.

@rmens

rmens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 1 minute.

@rmens

rmens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@package.json`:
- Line 15: Update the lint:fix script in package.json to chain vendor/bin/phpcbf
and biome check --write . with failure propagation, ensuring the command exits
unsuccessfully when PHPCBF fails while preserving the existing Biome formatting
step on successful PHPCBF execution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 95fa30b4-acf2-4015-83ac-35db0e06ad9e

📥 Commits

Reviewing files that changed from the base of the PR and between a7ef02b and e04a0be.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (13)
  • .github/workflows/js-lint.yml
  • README.md
  • biome.json
  • package.json
  • resources/css/admin.css
  • resources/css/tinymce-content.css
  • resources/ts/modules/utils.ts
  • resources/ts/tinymce-separator.js
  • tests/e2e/global-setup.ts
  • tests/e2e/helpers.ts
  • tests/e2e/interactions.spec.ts
  • tests/e2e/media.spec.ts
  • tsconfig.json

Comment thread package.json Outdated
@rmens

rmens commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 22 minutes.

@oszuidwest oszuidwest deleted a comment from coderabbitai Bot Jul 29, 2026
@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai I fact-checked the non-inline Docstring Coverage warning and disagree with adding metric-driven comments here. This PR adds CI/tooling configuration, package scripts, formatting/type annotations, and Playwright/Bun test code; it does not add a public application API that needs contract documentation. The repository enforces PHPCS, Biome, TypeScript, PHPStan, and behavioral tests, but has no 80% docstring threshold. The changed callbacks are local test/compatibility implementation details, so boilerplate docstrings would reduce signal without documenting a meaningful invariant. The existing targeted README and compatibility comments are the appropriate documentation for this change.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Tip

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

@rmens You’re right. The docstring-coverage warning was not appropriate for this PR: these changes do not introduce public application contracts, and adding boilerplate documentation to local test/compatibility callbacks would not improve maintainability. The existing README and targeted compatibility comments provide the relevant documentation. I’m withdrawing that finding.

🐇💭


✏️ Learnings added
Learnt from: rmens
URL: https://github.com/oszuidwest/teksttv-wp-plugin/pull/71

Timestamp: 2026-07-30T08:10:24.858Z
Learning: For the TekstTV WordPress plugin, do not request metric-driven docstrings for local test callbacks, tooling/configuration code, or compatibility implementation details. Document meaningful public contracts and non-obvious invariants instead; targeted README and compatibility comments are preferred. The repository does not enforce an 80% docstring-coverage threshold.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 30 minutes.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/js-lint.yml:
- Line 42: Pin the Bun runtime used by the frontend lint gate by adding an
explicit supported version to the oven-sh/setup-bun@v2 configuration in the
workflow and declaring the same version in the project’s package metadata.
Ensure check:frontend runs with this identical pinned version rather than
latest.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 01dc4ebd-ee40-461e-aae8-d64ffa96ca7b

📥 Commits

Reviewing files that changed from the base of the PR and between a7ef02b and 7748771.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (13)
  • .github/workflows/js-lint.yml
  • README.md
  • biome.json
  • package.json
  • resources/css/admin.css
  • resources/css/tinymce-content.css
  • resources/ts/modules/utils.ts
  • resources/ts/tinymce-separator.js
  • tests/e2e/global-setup.ts
  • tests/e2e/helpers.ts
  • tests/e2e/interactions.spec.ts
  • tests/e2e/media.spec.ts
  • tsconfig.json

Comment thread .github/workflows/js-lint.yml
@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai The fresh summary repeats two outside-inline warnings that do not warrant code changes:\n\n1. Docstring Coverage: this repository has no 80% docstring policy, and this PR adds tooling/configuration plus local test callbacks rather than new public application contracts. You previously agreed and withdrew this exact warning in https://github.com/oszuidwest/teksttv-wp-plugin/pull/71#issuecomment-5128284051.\n2. Linked Issues / lockfile: the check is inconclusive only because CodeRabbit excludes bun.lock through its own !**/*.lock path filter. The reviewed package.json pins typescript to exact 7.0.2, bun.lock contains the same exact resolution, and bun install --frozen-lockfile passes. That exclusion is an analyzer limitation, not a determinism gap in the PR.\n\nPlease withdraw both summary warnings.

@coderabbitai

coderabbitai Bot commented Jul 30, 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

printf '%s\n' '--- package.json TypeScript declaration ---'
rg -n -C 2 '"typescript"' package.json

printf '%s\n' '--- bun.lock TypeScript entries ---'
if [ -f bun.lock ]; then
  rg -n -C 2 'typescript@|typescript":' bun.lock | head -n 80
else
  printf '%s\n' 'bun.lock is not present in the checked-out PR.'
fi

printf '%s\n' '--- CodeRabbit configuration files and lockfile filters ---'
fd -HI -t f '(^|/)(\.coderabbit\.ya?ml|coderabbit\.ya?ml)$|^\.coderabbit/' . 2>/dev/null || true
rg -n -i -C 3 'path_filters|\.lock|coderabbit' \
  -g '.coderabbit.yml' -g '.coderabbit.yaml' -g 'coderabbit.yml' -g 'coderabbit.yaml' . 2>/dev/null || true

Length of output: 2803


@rmens You’re correct. I’ve withdrawn both summary warnings:

  1. Docstring Coverage — inapplicable for this tooling/configuration and local test/compatibility work; no repository policy requires the asserted coverage threshold.
  2. Linked Issues / lockfile — not a determinism gap. package.json pins TypeScript exactly to 7.0.2 (Line 39), and bun.lock resolves exactly typescript@7.0.2 (Line 718). The inability to inspect an analyzer-excluded lockfile is a review limitation, not a required code change.

🐇💭

You are interacting with an AI system.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 35 minutes.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@rmens

rmens commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai The new full review run c75619fc-e4bf-4c01-af42-91c47102fc58 again shows the outside-diff Docstring Coverage warning. I disagree for the same fact-checked reasons you already accepted and explicitly withdrew at #71 (comment): this PR changes CI/tooling configuration, package scripts, formatting/type annotations, and local Playwright/Bun test helpers, not public application contracts. The repository has no 80% docstring gate, and adding boilerplate comments to local callbacks would reduce signal rather than document a meaningful invariant. Please withdraw the repeated warning for this round as well.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Tip

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

@rmens You’re right. I’m withdrawing the repeated Docstring Coverage warning for this review round. This PR is limited to frontend CI/tooling, scripts, formatting/type adjustments, and local test helpers—not new public application contracts—and the repository has no docstring-coverage gate. Adding boilerplate documentation here would not add meaningful maintenance value.

🐇💭

You are interacting with an AI system.

@rmens
rmens merged commit 42bc904 into main Jul 30, 2026
10 of 11 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@biome.json`:
- Around line 4-5: Update the Biome files.includes configuration to explicitly
include the repository’s maintainer configuration files, including the root
package.json, biome.json, and TypeScript configuration files, so biome check .
covers them. Preserve the existing resources, tests, and Playwright entries, and
use the intended explicit config paths rather than leaving them unintentionally
excluded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d9f001b5-fc30-4690-812b-b8a77247cd78

📥 Commits

Reviewing files that changed from the base of the PR and between afc91a2 and 49a833b.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • .github/workflows/js-lint.yml
  • .github/workflows/release.yml
  • README.md
  • bin/package-plugin.sh
  • biome.json
  • package.json
  • tsconfig.json
  • tsconfig.tests.json

Comment thread biome.json
Comment on lines +4 to 5
"includes": ["resources/**", "tests/**", "playwright.config.ts"]
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | sed -n '1,200p'

echo
echo "biome.json:"
if [ -f biome.json ]; then cat -n biome.json | sed -n '1,220p'; fi

echo
echo "Search for files.includes / lint / check:"
rg -n '"files"|"includes"|"biome check"|lint|formatter' -S . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | sed -n '1,240p'

Repository: oszuidwest/teksttv-wp-plugin

Length of output: 8170


🌐 Web query:

Biome configuration files includes files includes configuration files biome check does not lint included linter rules

💡 Result:

In Biome, the files.includes and tool-specific .includes (such as linter.includes) fields determine which files are processed [1][2]. The statement that "biome check does not lint included linter rules" is not a standard or documented behavior of Biome, but users may encounter issues where files or rules appear ignored due to configuration order, glob syntax, or specific environment factors [3][4][5]. Key points regarding Biome's include/exclude configuration: 1. Relationship between files.includes and linter.includes: The global files.includes setting is the primary filter. If a file is not matched by files.includes, it is excluded from all Biome operations, including linting [6]. Consequently, a tool-specific include setting (like linter.includes) cannot include a file that has already been excluded by files.includes [2][6]. 2. Configuration Precedence and Overrides: Settings are hierarchical. If you are not seeing expected linting behavior, ensure that your glob patterns are correctly defined relative to the configuration file [2][7]. Additionally, there have been confirmed bug reports regarding inconsistent behavior with linter.includes in monorepo setups or when using specific CLI flags like --staged [3][5]. 3. Glob Pattern Requirements: When using negated patterns (e.g., !file.js) to exclude files, you must generally specify a base glob like `` first to ensure all files are initially considered [1][2]. For directory exclusions in tool-specific fields (e.g., linter.includes), using `!/path/to/dir/` is recommended to ensure the directory is properly excluded [8]. 4. Troubleshooting: If rules appear to be ignored: - Verify that your glob patterns are matching the intended files using documentation references [2]. - Check if the files are being unintentionally excluded by a broader `files.includes` pattern [6]. - Ensure you are not running into known issues regarding CLI execution paths or monorepo configurations, which can cause paths to be interpreted differently than expected [3][5]. For the most accurate debugging, ensure your `biome.json` is at the project root or correctly relative to your command execution, and verify the pattern syntax adheres to the current version's requirements [2][3][7].

Citations:


Include the maintainer configuration files in Biome’s coverage.

biome check . is the repo’s JS lint entry point, but files.includes only covers resources/**, tests/**, and playwright.config.ts. This leaves root package.json/biome.json and TS config files out of Biome’s lint/filter scope; add the intended config paths explicitly or document them as intentionally unlinted.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@biome.json` around lines 4 - 5, Update the Biome files.includes configuration
to explicitly include the repository’s maintainer configuration files, including
the root package.json, biome.json, and TypeScript configuration files, so biome
check . covers them. Preserve the existing resources, tests, and Playwright
entries, and use the intended explicit config paths rather than leaving them
unintentionally excluded.

@rmens
rmens deleted the ci/deterministic-frontend-checks branch July 30, 2026 16:23
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.

Make frontend lint, typecheck, and unit-test CI deterministic

1 participant