fix(devx): R3-1 - one truthful build path for every test runner - #422
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds a repository-root pre-push validation hook, centralizes stale Hugo builds across test scripts, improves changed-file detection, documents the script inventory and test gates, and adds guard tests for build and hook behavior. ChangesBuild and validation workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
All four runners (test/qtest/dtest/dtest-all) now build through a shared bin/build-if-stale helper that routes via bin/hugo-build (PurgeCSS cold-start warm-up guard) and skips the rebuild on a warm tree: - bin/test: DEST honors a caller-preset HUGO_DEFAULT_PATH and the build is skipped entirely under PRECOMPILED_ASSETS - un-breaks bin/dtest, whose container has no hugo and points at _dest/public-dtest - bin/qtest: shares bin/test's _dest/public-test tree (warm reruns skip the ~26s rebuild it previously paid on every micro-commit), and --changed now counts untracked files (a brand-new pages/foo.css no longer exits 0 green with 'nothing to run') - staleness list gains assets/ static/ package.json bun.lockb - .githooks/pre-push: run each unit-test file in its own ruby invocation (ruby file1 file2 executes only file1) - and commit the hook itself: the .* gitignore rule had kept it out of the repo, so fresh clones got a hooksPath pointing at nothing (added !.githooks) - bin_scripts_test: 3 new guards - no bare 'hugo --environment production' outside hugo-build/build-if-stale, pre-push multi-file ruby anti-pattern, bin/test HUGO_DEFAULT_PATH contract Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH
bin/README.md listed four scripts that do not exist (debug, benchmark-dev, bunx, generate-cover-image) and omitted eleven that do (qtest, dtest-all, setup-test-env, build-if-stale, hugo-build, ...). Rewritten to match the actual directory contents. .okf test-gates: drop the stale 'CI does NOT run screenshot diffs' claim (superseded by #413/#417), record the shared build-if-stale warm-tree flow, qtest untracked-file detection, and the .gitignore dot-path gotcha that had kept the pre-push hook out of the repo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH
7a106a7 to
36b49ab
Compare
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.githooks/pre-push:
- Around line 27-32: Replace the direct Ruby invocations for
toolchain_pins_test.rb and bin_scripts_test.rb in the pre-push hook with a
repository-owned guard test task, and invoke that task through the project’s
bin/rake or another established test entrypoint. Add the task to the project’s
test configuration, then preserve the existing failure handling and descriptive
fail messages.
In `@bin/build-if-stale`:
- Around line 21-23: Update the stale-check logic around newest_src to detect
deletions within the watched source trees, rather than relying only on existing
files. Include directory timestamps or use a successful-build manifest so
removing a watched file marks the output stale, and add a regression test
covering deletion of a watched source file.
In `@test/unit/bin_scripts_test.rb`:
- Around line 47-78: Replace the source-text assertions in
test_no_bare_production_hugo_builds_outside_hugo_build,
test_pre_push_hook_runs_each_test_file_separately, and
test_bin_test_honors_preset_hugo_default_path with behavior-focused executions
using controlled commands or stubs. Assert the observed Hugo build routing, that
the pre-push hook runs each test file, and that bin/test uses a caller-provided
HUGO_DEFAULT_PATH, without relying on script text, file existence, assignments,
or invocation-pattern checks.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f16cac6-c71a-4d93-823d-9828a35cceb3
📒 Files selected for processing (11)
.githooks/pre-push.gitignore.okf/build/test-gates.md.okf/log.mdbin/README.mdbin/build-if-stalebin/dtestbin/dtest-allbin/qtestbin/testtest/unit/bin_scripts_test.rb
- New `rake test:guards` task (toolchain pins + bin scripts, ~2s) is the pre-push hook's single test entrypoint - repo-provided command instead of direct ruby file invocations, and structurally immune to the multi-file-ruby trap - bin/build-if-stale staleness probe now includes directory mtimes: deleting a source file leaves nothing to be -newer, but bumps its parent dir - verified warm tree goes stale on a removed content file - pre-push guard test asserts the rake entrypoint is present (no longer passes vacuously if the hook drops its test invocation) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH
Correctness: - hugo_stats.json gets its own EXACT-key-only cache (no restore-keys): a partial hit restored a stale stats file from an older tree, and the warm-up skip trusted it - PurgeCSS could purge classes live in the current tree (the 2026-07-19 .sr-only incident class, re-opened via cache). Exact hit = identical tree = valid stats - test.yml PR runs check out the MERGE commit (default checkout); head_ref tested the tip without the base and breaks fork PRs. Record dispatches still check out the branch ref (needed for the push) - record mode runs rake test:system (not the full suite - a unit failure used to abort AFTER 15+ min of recording), and the commit step is always()-gated + porcelain-guarded so recorded PNGs land even when one page's test is red - visual + link-check paths filters gain root layouts/** and assets/** (root-level overrides skipped both gates) Cost: - sync fan-out gated: publish.yml and link-check.yml workflow_run jobs skip when the sync pushed nothing (head_sha == current master head) - 'Sync articles' fires every 10 min 8-21 UTC and mostly commits nothing; that was ~84 no-op deploy+test cascades per day - _dest dropped from the setup-hugo cache (~1-2 GB per job, evicting every other cache from the 10 GB quota; the build regenerates it) - link-check builds ONCE: composite runs with build:'false', rake test:links does its own production build - the double build blew the 10-minute timeout on cold caches (seen live on PR #422) Claude-Session: https://claude.ai/code/session_01PXHeUErqoiyH9xN1mjC8cH Co-authored-by: Claude <noreply@anthropic.com>
What
Round-3 DevX: make the local test harness truthful and fast. All four test runners (
bin/test,bin/qtest,bin/dtest,bin/dtest-all) now build through one shared helper, and the lies the multi-agent audit found are fixed:bin/build-if-stale <dest> [base-url]: routes every test build throughbin/hugo-build(PurgeCSS cold-start warm-up guard — the 2026-07-19.sr-onlyincident class) and skips the rebuild when the dest tree is newer than every source tree. Staleness probe now also watchesassets/,static/,package.json,bun.lockb(previously blind to badge/static-asset edits).FORCE_BUILD=1overrides.bin/testno longer breaksbin/dtest:DESThonors a caller-presetHUGO_DEFAULT_PATH, and the build is skipped entirely whenPRECOMPILED_ASSETSis already set — previously it exported_dest/public-testover the container's_dest/public-dtestand pointed the Docker suite at the wrong tree (orhugo: command not foundon a stale one).bin/qtestsharesbin/test's warm tree (_dest/public-test): the per-micro-commit gate stops paying a ~26s private rebuild on every invocation (measured warm skip: 0.02s).bin/qtest --changedcounts untracked files: a brand-newpages/foo.csspreviously produced "no visual-affecting changes" and a false green exit 0.rubyinvocation (only the first executes — the guard suite was silently skipped), now split into two invocations. Bigger find: the hook file itself had never been committed — the.gitignore.*rule swallowed.githooks/, so every fresh clone hadcore.hooksPathpointing at nothing. Added the!.githooksnegation and committed the hook.bin_scripts_testguards: no barehugo --environment productionoutsidehugo-build/build-if-stale; pre-push must not pass multiple test files to oneruby;bin/testmust defaultDESTfromHUGO_DEFAULT_PATH.bin/README.mdlisted four scripts that don't exist and omitted eleven that do — rewritten to match the directory..okftest-gates concept updated (drops the stale "CI does NOT run screenshot diffs" claim, superseded by ci(visual): R2 Phase B - screenshot tests on the pinned rendering stack + report-only PR gate #413/fix(ci): make the visual harness truthful - build drafts, allow first baselines #417) + log entry._dest/public-test-[0-9]trees left by the oldrand(5)bucket fan-out (local cleanup, not in the diff).Verification
bundle exec rake test:unit: 278 runs, 0 failures (275 pre-existing + 3 new guards); new guards verified red against the reverted patterns.bin/build-if-stale _dest/public-testtwice — second run skips in 0.02s; touching a content file correctly re-triggers a build.touch themes/beaver/assets/css/pages/zzz-fake.css→ qtest sees it and aborts loudly (exit 1) instead of exiting 0 green.bin/qtest homepageend-to-end on the pinned rendering stack: reused the warm tree, ran the scoped tests, and the dirty-baseline guard fired as designed. (The screenshot diffs themselves are the known old-baselines-vs-pinned-stack gap that the post-fix(ci): rebase before pushing recorded baselines (push race) #418 baseline re-record closes — environmental, reproduced identically in CI on master.)bash -n/ruby -cclean on all edited scripts; pre-push hook ran green on the push of this branch.Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests