Skip to content

[codex] fix rule path validation for custom summary rules#944

Draft
MickeyWzt wants to merge 2 commits into
ev-flow:masterfrom
MickeyWzt:fix-rule-default-path-validation
Draft

[codex] fix rule path validation for custom summary rules#944
MickeyWzt wants to merge 2 commits into
ev-flow:masterfrom
MickeyWzt:fix-rule-default-path-validation

Conversation

@MickeyWzt

Copy link
Copy Markdown

Summary

Fixes #936.

This delays validation of the default rules directory until the CLI actually needs to walk a rules directory. Commands that provide a specific JSON rule with -s custom_rule.json no longer fail during Click option parsing just because the default rules directory has not been downloaded yet.

Root cause

The --rule option used click.Path(exists=True) with a default path. Click validates default values while parsing, so an absent default rules directory blocked commands even when the command used a specific rule file instead.

Changes

  • Remove eager exists=True validation from the --rule Click option.
  • Add an explicit missing-directory check before walking the rules directory.
  • Add a regression test that sets the default rule path to a missing directory and verifies a custom summary rule file is still accepted.

Validation

  • python -m pytest tests/test_cli.py -q passed.
  • python -m pytest tests/test_cli.py tests/test_freshquark.py -q shows the new test passing, while the existing tests/test_freshquark.py tests fail on Windows path separator expectations (/ vs \\), unrelated to this change.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.89%. Comparing base (1df1d49) to head (2802f3c).

Files with missing lines Patch % Lines
quark/cli.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #944   +/-   ##
=======================================
  Coverage   78.89%   78.89%           
=======================================
  Files          81       81           
  Lines        7131     7131           
=======================================
  Hits         5626     5626           
  Misses       1505     1505           
Flag Coverage Δ
unittests 78.89% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MickeyWzt MickeyWzt force-pushed the fix-rule-default-path-validation branch from db840f6 to a00321f Compare June 30, 2026 14:12
@MickeyWzt MickeyWzt force-pushed the fix-rule-default-path-validation branch from a00321f to 2802f3c Compare June 30, 2026 14:50
@MickeyWzt

Copy link
Copy Markdown
Author

I added a second regression test for the missing --rule directory branch and verified the focused CLI tests locally:

python -m pytest tests/test_cli.py -q
# 2 passed

One note on the Codecov patch failure: this looks like a workflow checkout issue for fork PRs rather than a missing test. The build workflow runs on pull_request_target, and the Actions log for the previous run shows actions/checkout fetching and checking out the base commit 1df1d49... from origin/master, while Codecov uploads that coverage report against the PR head commit. That means the uploaded coverage is generated from the base branch, not from this PR's changed files, so patch coverage reports the changed quark/cli.py lines as uncovered even though the new tests cover the added branch when run against the PR branch.

The latest push restores the source diff to the clean implementation and keeps the added regression tests focused on the behavior in this PR.

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.

--rule default path validated even when unused

1 participant