Modernize testing and CI: drop change-detector meta-tests, dedupe CI jobs - #404
Merged
Conversation
Delete the test-suite layers that asserted repository *text* rather than behavior: - test_ci_workflow.py: 927 lines re-asserting the literal YAML of the workflows (SHA pins, persist-credentials, template shapes are already enforced by zizmor + actionlint in pre-commit) and the contents of plan/validation/tasks.md. - test_source_hygiene.py: asserted the source text of other test files (exact asyncio.sleep counts, required helper names) and planning docs. - test_module_size_ratchet.py: line-count ratchet; ruff C901/PLR0912/ PLR0915 remain as the complexity gate. - test_validation_plan.py and every embedded test_validation_tasks_v*/test_*_plan_tracks_* change detector: these pinned backtick-quoted phrases in plan/ markdown, turning every code change into a mandatory plan-doc edit. - test_stress_uses_public_sampler.py: AST-parsed a sibling test to police its imports. - tests/regressions/: empty placeholder directory. Guards that protect user-facing surfaces (docs routes, command hints, generated files, packaging hygiene, marker lint) are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
- Merge the duplicated lint + pre-commit jobs into a single pre-commit gate: ruff check/format, Import Linter, actionlint, zizmor, codespell, and whitespace hooks now run once, identically in CI and locally. - Trim the per-PR quick-validation matrix to the oldest and newest supported Pythons (3.11/3.14); 3.12 keeps per-PR coverage via the coverage job, and the nightly full-local job now runs the full 3.11-3.14 matrix under the thorough Hypothesis profile. - Drop the nightly quick job: it was a strict subset of nightly full-local and already runs on every PR and in the release gate. - Drop tests/test_validation_plan.py from the guard-contributing and guard-validation recipes and regenerate the guard command surfaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR consolidates CI and validation execution, updates guard-command docs and recipes, removes validation-plan cross-check tests, and applies typing and helper refactors across core, telephony, transport, and validation code. ChangesCI, validation, and guard execution
Plan-tracking test cleanup
Release artifact scaffold exemption
Core helpers and registries
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: Stream initialization permanently failed: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
yisding
marked this pull request as ready for review
July 8, 2026 14:56
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CONTRIBUTING.md (1)
111-112: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale references to removed plan-tracking coverage.
guard-contributingandguard-validationno longer runtests/test_validation_plan.py(per the raw fallback changes at Line 77-78), but this table still describes them as protecting "plan current-state evidence" and "validation plan current state." Update this prose to match the new guard scope.📝 Proposed fix
-| Contributor and validation guidance | `just guard-contributing` | `justfile` parity, agent guide command, source-layout, and architecture hints, validation lanes, docs-route hints, and plan current-state evidence | -| Validation workflow, validation reference, or validate CLI behavior | `just guard-validation` | The `docs/validation.md` workflow, validation reference route hints, validation plan current state, validate CLI reports, JSON envelopes, latency options, and error handling | +| Contributor and validation guidance | `just guard-contributing` | `justfile` parity, agent guide command, source-layout, and architecture hints, validation lanes, and docs-route hints | +| Validation workflow, validation reference, or validate CLI behavior | `just guard-validation` | The `docs/validation.md` workflow, validation reference route hints, validate CLI reports, JSON envelopes, latency options, and error handling |🤖 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 `@CONTRIBUTING.md` around lines 111 - 112, Update the CONTRIBUTING guidance table so the descriptions for guard-contributing and guard-validation no longer mention removed plan-tracking coverage. Adjust the prose to reflect the current scope of the guard commands, using the existing guard names and the validation workflow references, and remove any phrasing about “plan current-state evidence” or “validation plan current state.”
🤖 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.
Outside diff comments:
In `@CONTRIBUTING.md`:
- Around line 111-112: Update the CONTRIBUTING guidance table so the
descriptions for guard-contributing and guard-validation no longer mention
removed plan-tracking coverage. Adjust the prose to reflect the current scope of
the guard commands, using the existing guard names and the validation workflow
references, and remove any phrasing about “plan current-state evidence” or
“validation plan current state.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5405727b-77ad-4785-9f51-0ca157876b88
📒 Files selected for processing (37)
.github/workflows/ci.yml.github/workflows/nightly-validation.ymlAGENTS.mdCLAUDE.mdCONTRIBUTING.mddocs/validation.mdjustfilellms-full.txtsrc/easycat/cli/_guard_commands.pytests/_release_artifacts.pytests/cli/test_app.pytests/cli/test_bundles.pytests/cli/test_doctor.pytests/cli/test_latency_baseline_budgets.pytests/cli/test_latency_reliability_failures.pytests/cli/test_latency_selectors_artifacts.pytests/cli/test_templates.pytests/cli/test_validate_live.pytests/cli/test_validate_report_model.pytests/cli/test_validate_runner.pytests/contracts/test_agent_bridge_contracts.pytests/contracts/test_http_cassette_redaction.pytests/contracts/test_provider_capability_report_model.pytests/contracts/test_provider_capability_reports.pytests/contracts/test_provider_surface_matrix.pytests/contracts/test_ws_cassette_replay.pytests/observability/test_attributes.pytests/perf/test_journal_benchmark_artifact.pytests/regressions/.gitkeeptests/test_ci_workflow.pytests/test_contributing.pytests/test_dx_helpers.pytests/test_module_size_ratchet.pytests/test_source_hygiene.pytests/test_validation_markers.pytests/test_validation_plan.pytests/validation/test_stress_uses_public_sampler.py
💤 Files with no reviewable changes (26)
- tests/cli/test_templates.py
- tests/cli/test_bundles.py
- tests/cli/test_validate_runner.py
- tests/contracts/test_provider_capability_reports.py
- tests/cli/test_latency_baseline_budgets.py
- tests/validation/test_stress_uses_public_sampler.py
- tests/test_validation_plan.py
- tests/perf/test_journal_benchmark_artifact.py
- tests/test_module_size_ratchet.py
- tests/test_ci_workflow.py
- tests/cli/test_validate_report_model.py
- tests/cli/test_doctor.py
- tests/contracts/test_ws_cassette_replay.py
- tests/contracts/test_agent_bridge_contracts.py
- tests/test_contributing.py
- tests/test_source_hygiene.py
- tests/cli/test_latency_selectors_artifacts.py
- tests/contracts/test_provider_surface_matrix.py
- tests/cli/test_app.py
- tests/cli/test_validate_live.py
- tests/contracts/test_provider_capability_report_model.py
- tests/cli/test_latency_reliability_failures.py
- tests/test_dx_helpers.py
- tests/_release_artifacts.py
- tests/observability/test_attributes.py
- tests/contracts/test_http_cassette_redaction.py
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Non-blocking feedback (1)
- Guard coverage descriptions still mention removed plan-tracking checks — CONTRIBUTING.md#L111-L112
The raw guard command lists were updated to droptests/test_validation_plan.py, but the “What it protects” column still saysguard-contributing/guard-validationcover “plan current-state evidence” and “validation plan current state.” Updating those two phrases to match the current guard scope would keep the contributor guidance consistent.
If you want Charlie to apply this, reply with please fix 1.
- Run the quick validation slice with pytest-xdist (-n auto --dist loadscope), matching the flags just test-fast already uses for the same marker expression; locally the lane drops from ~4min serial to ~48s. Socket, stress, contracts, and latency lanes stay serial. The release venv installs pytest-xdist so the wheel-gated quick slice keeps working. - Run the CI coverage job with the same xdist flags (pytest --cov is xdist-safe; matches just cov). - Drop the advisory whole-repo mypy CI step: continue-on-error output that nothing gates on is spent compute. just typecheck-all remains for local/on-demand ratcheting. - Skip the unnecessary dev sync in build-smoke (uv build resolves its backend in an isolated env). - Update the CONTRIBUTING guard table and xdist-safety prose to match the current guard scope and parallel lanes (flagged by review bots). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
…leted Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
There was a problem hiding this comment.
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/ci.yml:
- Around line 60-67: The advisory whole-repo mypy signal was removed from the CI
workflow, so confirm whether non-gated packages should still be type-checked
somewhere. Update the CI configuration around the mypy step and the related
`just typecheck`/`just typecheck-all` workflow so the intended coverage remains
available, either by restoring an advisory `continue-on-error` pass or by
documenting/moving that signal to a separate job such as nightly.
🪄 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: 5bbbd4f8-1159-4c43-949e-b43fe5676130
📒 Files selected for processing (6)
.github/workflows/ci.yml.github/workflows/release-validation.ymlCONTRIBUTING.mdjustfilesrc/easycat/validation/runner.pytests/cli/test_validate_runner.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nightly-validation.yml (1)
23-26: 🧹 Nitpick | 🔵 TrivialMatrix expansion quadruples nightly full-local runner time.
Moving from a single 3.12 run to a 4-version matrix (
fail-fast: false, 45-min timeout each) meaningfully increases nightly CI minutes. This appears intentional per the added rationale comment, but worth keeping in mind for runner budget/cost tracking.🤖 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 @.github/workflows/nightly-validation.yml around lines 23 - 26, The nightly-validation workflow’s Python version matrix is expanding the full-local job cost too much; adjust the strategy in the matrix job so it does not run all four versions every night. Either reduce the python-version set to a single representative version or split the broader matrix into a separate less-frequent workflow, keeping the existing job structure and fail-fast behavior intact.
🤖 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.
Outside diff comments:
In @.github/workflows/nightly-validation.yml:
- Around line 23-26: The nightly-validation workflow’s Python version matrix is
expanding the full-local job cost too much; adjust the strategy in the matrix
job so it does not run all four versions every night. Either reduce the
python-version set to a single representative version or split the broader
matrix into a separate less-frequent workflow, keeping the existing job
structure and fail-fast behavior intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f243c3a0-bd82-4f46-ae68-9db46e3e0b16
📒 Files selected for processing (2)
.github/workflows/nightly-validation.ymltests/cli/test_templates.py
Fix all 85 real mypy errors outside the vendored FunASR runtime — stale annotations (a 4-tuple list typed as 3-tuples in webrtc), str locals that should carry Literal types (validation statuses, telephony platform/result values, server state), lazily-imported optional module handles inferred as object/Any|None, registry value types for the provider factories, and declared-but-dynamic Session attributes. No runtime behavior changes; a handful of casts state invariants that existing runtime guards already establish, and no new type: ignore comments were added. The vendored vad/_funasr_runtime is excluded via a pyproject override (hand-annotating vendored numeric code is churn without value). With the count at zero, the advisory/gated split is gone: just typecheck now runs mypy over the whole package and CI gates on it. The former clean-core module list stays as a strict override (check_untyped_defs, warn_unused_ignores) to keep ratcheting, and the now-redundant typecheck-all recipe and mypy_gated_paths variable are removed along with their sync guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
There was a problem hiding this comment.
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 `@CONTRIBUTING.md`:
- Around line 51-52: The Type gate row in CONTRIBUTING.md overstates coverage by
labeling `just typecheck` as “whole repo” while the referenced command `uv run
mypy src/easycat` only checks the package tree. Update the wording to match the
actual scope, or expand the `just typecheck` target/underlying command so it
באמת covers the full repository; use the `just typecheck` and `uv run mypy
src/easycat` entries as the anchors when editing the table.
🪄 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: 3f6967d7-7bb9-47da-b12e-060cacb19672
📒 Files selected for processing (42)
.github/workflows/ci.ymlCONTRIBUTING.mdjustfilepyproject.tomlsrc/easycat/_logging.pysrc/easycat/_observability.pysrc/easycat/_provider_catalog.pysrc/easycat/cli/debug/bundles.pysrc/easycat/cli/debug/replay.pysrc/easycat/cli/validate.pysrc/easycat/config/_telephony_wiring.pysrc/easycat/config/easy.pysrc/easycat/echo_cancellation.pysrc/easycat/noise_reduction.pysrc/easycat/planning/provider_plan.pysrc/easycat/server/auth.pysrc/easycat/server/routes.pysrc/easycat/server/voice_server.pysrc/easycat/server/webrtc_routes.pysrc/easycat/session/_session.pysrc/easycat/strip_markdown.pysrc/easycat/stt/base.pysrc/easycat/stt/factory.pysrc/easycat/supervisor.pysrc/easycat/telephony/outbound.pysrc/easycat/telephony/screening.pysrc/easycat/telephony/server.pysrc/easycat/telephony/voicemail.pysrc/easycat/testing/contracts.pysrc/easycat/transports/local.pysrc/easycat/transports/twilio_media.pysrc/easycat/transports/webrtc.pysrc/easycat/transports/webtransport.pysrc/easycat/tts/_ws_base.pysrc/easycat/tts/factory.pysrc/easycat/tts/input.pysrc/easycat/vad/_base.pysrc/easycat/validation/latency.pysrc/easycat/validation/redaction.pysrc/easycat/validation/runner.pytests/test_contributing.pytests/test_dependency_policy.py
The gate runs mypy over src/easycat, not tests/ or scripts/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
yisding
pushed a commit
that referenced
this pull request
Jul 8, 2026
Merge current main into the PR branch after #404 restructured the CI workflows, re-pinning every setup-uv usage to v8.2.0. Resolves the ci.yml / nightly-validation.yml conflicts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TNSNVmxYCC9vQmEfo2xHsa
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes the layers of the test suite that asserted repository text rather than behavior (~4,200 lines), and consolidates the CI workflows so nothing runs twice per PR.
Tests removed (change detectors, tests-of-tests, plan-doc pins)
tests/test_ci_workflow.py(927 lines) — re-asserted the literal YAML of the workflows and the contents ofplan/validation/tasks.md. The real security posture (SHA-pinned actions,persist-credentials: false, template injection, cache poisoning) is already enforced by zizmor + actionlint, which run in pre-commit and CI.tests/test_source_hygiene.py(896 lines) — asserted the source text of other test files (exactasyncio.sleep(counts, required helper names) and planning markdown.tests/test_module_size_ratchet.py— line-count ratchet with a shrink-only allowlist; ruff's C901/PLR0912/PLR0915 complexity gates remain.tests/test_validation_plan.pyplus 31 embeddedtest_validation_tasks_v*/test_*_plan_tracks_*detectors across 18 files (cli, contracts, observability, perf, dx-helpers, contributing, validation-markers) — each pinned backtick-quoted phrases insideplan/**markdown, so routine code changes forced edits to historical planning docs.tests/validation/test_stress_uses_public_sampler.py— AST-parsed a sibling test file to police its imports.tests/regressions/placeholder directory and the now-unusedREQUIRED_BUILD_SOURCE_EXCLUDESconstant (wheel/sdist contents are still checked directly bytests/cli/test_packaging.py).Kept intact: every guard that protects a user-facing surface — docs routes and command hints, generated files (
llms.txt, guard-command blocks), packaging/scaffold hygiene, marker lint, dependency policy, and the contract suites.CI consolidation
lintandpre-commitjobs duplicated each other (ruff check/format, Import Linter, and zizmor each ran twice per PR).lintnow runspre-commit run --all-fileswith no skips, so CI and the local hook grade the exact same surface. Note: if branch protection listspre-commitas a required check, it should be updated tolint.full-localjob now runs the full 3.11–3.14 matrix under the thorough Hypothesis profile — so intermediate versions get deeper coverage than before, off the PR critical path.quickjob was a strict subset of nightlyfull-localand already runs on every PR and in the release gate; removed.justfileno longer reference the deleted plan test; all generated surfaces (CLAUDE.md,AGENTS.md,CONTRIBUTING.md,docs/validation.md,llms-full.txt,_guard_commands.py) regenerated viascripts/regen_guard_commands.py.Net effect per PR: 4 fewer duplicated job-runs of lint tooling, 2 fewer quick-validation matrix legs, and a test suite that no longer fails when someone edits a planning doc, a comment, or a workflow without updating a mirror assertion.
Testing
uv run pytest— 5478 passed, 319 skipped in 4m06s.uv run pre-commit run --all-files— all hooks pass (zizmor run offline locally due to sandbox network; clean, no findings).just typecheck(gated mypy) — clean.tests/docs,test_contributing.py,test_regen_guard_commands.py,test_llms_txt.py, packaging) plus all 18 trimmed test files — green.🤖 Generated with Claude Code
https://claude.ai/code/session_01Nj7UG2gQfxqau6c9RdyKKK
Summary by CodeRabbit
doctor --fixnow creates the journal directory automatically when it doesn’t exist.--releaseflag.-n auto --dist loadscope).