Skip to content

[COVAL-4319] Open weekly CLI API parity PRs and gate releases - #97

Merged
callumreid merged 5 commits into
mainfrom
callum/coval-4319-weekly-cli-parity
Jul 31, 2026
Merged

[COVAL-4319] Open weekly CLI API parity PRs and gate releases#97
callumreid merged 5 commits into
mainfrom
callum/coval-4319-weekly-cli-parity

Conversation

@callumreid

@callumreid callumreid commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • strengthen the live OpenAPI audit from client-route presence to first-class command exposure, including checked snapshot drift
  • generate a deterministic, timestamp-free api-coverage-report.md
  • open or update one rolling weekly parity PR when API or CLI coverage changes
  • reserve the reusable GitHub issue for automation failures where no PR could be produced
  • pin and run Python automation tests, Ruff, the live audit, and report freshness in CI
  • add tested Cargo version bump/validation and Homebrew formula rendering helpers
  • gate automatic tags on the exact successful main CI run, then call the reusable release workflow directly
  • make Homebrew updates retry-safe and reuse one release failure issue

Repository-owned cadence

GitHub Actions runs every Monday at 10:00 UTC. It refreshes api-coverage-report.md and uses peter-evans/create-pull-request with the stable chore/weekly-api-parity branch, so ordinary drift opens or updates one PR instead of filing an issue.

The generated PR is deliberately honest about the CLI's hand-written command UX. If the report says ACTION REQUIRED, its strict CI remains red until first-class commands or an explicitly reviewed manifest exception reconcile the drift. A report-only red PR should not be merged.

The issue fallback is used only if fetching, generation, authentication, or PR creation fails. That matches the SDK workflow's split between a normal regeneration PR and an issue for broken automation.

Repository prerequisite

  • REGEN_PR_TOKEN: configured as a fine-grained token limited to coval-ai/cli, with Contents and Pull requests read/write access and a July 31, 2027 expiration. The organization does not allow the repository's current GITHUB_TOKEN setting to create pull requests.
  • HOMEBREW_TAP_TOKEN: Contents read/write access to coval-ai/homebrew-tap.

REGEN_PR_TOKEN was configured out of band; no credential value is stored in the repository or PR.

Release safety

  • no version bump means no release
  • an untagged version is tagged only after the exact main push CI succeeds
  • tag and Cargo manifest versions must match
  • manual dispatch reruns the existing current release without creating another tag
  • GITHUB_TOKEN creates the tag; the release workflow is invoked directly, avoiding workflow-recursion suppression without a personal release PAT
  • the human PR merge gate remains intact

Current parity

  • 174 published operations audited live
  • 124 have first-class CLI command coverage
  • 50 reviewed gaps remain under COVAL-2079
  • trace search is published and covered

Validation

  • actionlint -color
  • python3 -m ruff check scripts
  • python3 -m ruff format --check scripts
  • python3 -m unittest discover --start-directory scripts --pattern 'test_*.py' (31 tests)
  • deterministic report regeneration produced an identical SHA-256
  • python3 scripts/audit_api_coverage.py --write-markdown api-coverage-report.md (PASS)
  • python3 scripts/release_version.py --expected-tag v0.5.0
  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-targets --all-features (108 tests)

Rebased onto main after #96 merged. No personal automation, merge, tag, release, deploy, or secret mutation is included.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Adds trace search validation and CLI tests, a first-class API coverage audit with deterministic reporting, expanded CI checks, and scheduled parity pull-request automation. Introduces version bumping, release-version validation, Homebrew formula rendering, tag-based release workflows, and release failure recovery. Documentation and generated coverage data are updated to describe the new audit and release processes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% 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
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary changes: weekly CLI API parity pull requests and release gating.
Description check ✅ Passed The description directly explains the API parity automation, CI validation, release gating, and Homebrew updates in the changeset.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

@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: 2

🧹 Nitpick comments (9)
src/commands/traces.rs (1)

235-236: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider rejecting --duration-ms-min > --duration-ms-max locally.

Currently an inverted range is sent to the API and silently returns nothing (or a server-side error), which is a confusing UX for an easily detectable input mistake. Same pattern as the attribute-filter cap check below, so it can go right before input_json::finish.

🤖 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 `@src/commands/traces.rs` around lines 235 - 236, Add local validation in the
trace command flow before input_json::finish to reject cases where
args.duration_ms_min exceeds args.duration_ms_max, following the existing
attribute-filter cap validation pattern. Return a clear input error and preserve
the current filter insertion behavior for valid or partially specified ranges.
.github/workflows/api-parity-audit.yml (1)

26-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Disable credential persistence on actions/checkout in both workflows. Neither job needs the checked-out token to persist past checkout (no later push/pull using it), and both run pip install of third-party packages afterward, extending the token's exposure window unnecessarily.

  • .github/workflows/api-parity-audit.yml#L26-L26: add with: { persist-credentials: false } to the actions/checkout@v6 step.
  • .github/workflows/ci.yml#L19-L19: add with: { persist-credentials: false } to the actions/checkout@v6 step.
🔒 Proposed fix (apply to both)
       - uses: actions/checkout@v6
+        with:
+          persist-credentials: false
🤖 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/api-parity-audit.yml at line 26, Disable credential
persistence on the actions/checkout@v6 step in
.github/workflows/api-parity-audit.yml lines 26-26 and .github/workflows/ci.yml
lines 19-19 by configuring persist-credentials to false in each step; no other
workflow changes are needed.

Source: Linters/SAST tools

.github/workflows/ci.yml (1)

38-39: 🩺 Stability & Availability | 🔵 Trivial

Live external API call now gates every CI run, not just the weekly backstop.

audit_api_coverage.py fetches https://api.coval.dev/v1/openapi live on every PR/push via this check job. An outage, rate-limit, or slowdown of that endpoint would block all PR merges repo-wide, not only the scheduled parity check. The 3-attempt exponential-backoff retry in _fetch mitigates brief blips but not a sustained outage. Since this is an intentional design choice per the PR (live catalog as source of truth), just flagging the operational tradeoff — consider whether a short-circuit/soft-fail path is warranted if this proves flaky in practice.

🤖 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/ci.yml around lines 38 - 39, Adjust the “Audit live API
command coverage” step in the check job so failures from the live api.coval.dev
fetch do not block ordinary PR and push CI runs, while preserving strict
enforcement for the scheduled weekly parity check. Use the workflow’s event
context to apply the soft-fail or short-circuit behavior only to non-scheduled
runs.
scripts/test_audit_api_coverage.py (1)

1-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding coverage for _manifest_operations and audit() aggregation.

Current tests exercise fetch-safety and client/command mapping well, but nothing covers _manifest_operations's validation errors (missing reason, invalid method, duplicate operation) or audit()'s overlapping-manifest-section check and gap/extra classification — the logic that actually decides pass/fail for CI and the weekly backstop.

🤖 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 `@scripts/test_audit_api_coverage.py` around lines 1 - 170, Add focused tests
for _manifest_operations covering missing reasons, invalid HTTP methods, and
duplicate operations, asserting each validation failure. Add audit() tests for
overlapping manifest sections and for correctly classifying operation gaps and
extras, including the resulting pass/fail decision used by CI and the weekly
backstop. Reuse the existing unittest and mock patterns without changing
production behavior.
.github/workflows/release.yml (2)

32-44: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Add a least-privilege permissions block to validate.

The job only needs contents: read; without a block it inherits the workflow/repo default (or the caller's contents: write on the reusable path).

🔒️ Suggested change
   validate:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
       - uses: actions/checkout@v6
         with:
           ref: ${{ env.RELEASE_REF }}
+          persist-credentials: false
🤖 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/release.yml around lines 32 - 44, Add a job-level
permissions block to the validate job, granting only contents: read. Keep the
existing checkout, Python setup, and release-version validation steps unchanged.

Source: Linters/SAST tools


186-194: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Avoid persisting the tap PAT in the cloned repo's config.

The token is baked into the remote URL, so it stays in $tap_dir/.git/config for the remainder of the job and any later step or subprocess can read it. Use an auth header instead.

🔒️ Suggested change
           tap_dir="${RUNNER_TEMP}/homebrew-tap"
-          git clone "https://x-access-token:${HOMEBREW_TAP_TOKEN}`@github.com/coval-ai/homebrew-tap.git`" "$tap_dir"
+          auth_header="Authorization: Basic $(printf 'x-access-token:%s' "$HOMEBREW_TAP_TOKEN" | base64 -w0)"
+          git -c "http.https://github.com/.extraheader=$auth_header" \
+            clone "https://github.com/coval-ai/homebrew-tap.git" "$tap_dir"

Pass the same -c http.https://github.com/.extraheader=... to the push invocation.

🤖 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/release.yml around lines 186 - 194, Update the Homebrew
tap clone and push flow around tap_dir so HOMEBREW_TAP_TOKEN is supplied through
Git’s HTTPS extraheader authentication instead of embedded in the repository
URL. Use the same http.https://github.com/.extraheader configuration for both
clone and push, ensuring the token is not persisted in tap_dir/.git/config.
scripts/release_version.py (1)

14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated release-version pattern in two scripts. Both files define the identical VERSION_RE, so the release-version contract can drift between the validator and the formula renderer.

  • scripts/release_version.py#L14-L14: keep this as the single source of truth for VERSION_RE.
  • scripts/render_homebrew_formula.py#L11-L11: import VERSION_RE from release_version (using the same package/script import shim already present in scripts/bump_version.py Lines 10-15) instead of redefining it.
🤖 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 `@scripts/release_version.py` at line 14, Keep VERSION_RE in
scripts/release_version.py as the single source of truth. In
scripts/render_homebrew_formula.py, remove the duplicate pattern and import
VERSION_RE from release_version using the existing package/script import shim
demonstrated in scripts/bump_version.py; no direct change is needed in
release_version.py.
scripts/bump_version.py (1)

42-56: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Scope the manifest rewrites to the package entry and derive the crate name.

Two fragilities here:

  • The Cargo.toml pattern matches any version = "<current>" line; with count=1 the first match wins silently, so a dependency pinned to the same version appearing before [package].version would be rewritten instead.
  • The Cargo.lock pattern hardcodes name = "coval", while release_version() (scripts/release_version.py Line 19) resolves the name from Cargo.toml. A rename breaks the bump path only.
♻️ Suggested tightening
+import tomllib
+
 def bump_version(part: str, root: Path = ROOT) -> str:
     current = release_version(root)
     updated = next_version(current, part)
     cargo_path = root / "Cargo.toml"
     lock_path = root / "Cargo.lock"
+    name = tomllib.loads(cargo_path.read_text())["package"]["name"]
 
     cargo = _replace_once(
         cargo_path.read_text(),
-        rf'^version = "{re.escape(current)}"$',
-        f'version = "{updated}"',
+        rf'(^\[package\](?:\n(?!\[).*)*?\nversion = ")'
+        rf'{re.escape(current)}' + r'("$)',
+        rf"\g<1>{updated}\g<2>",
         cargo_path,
     )
     lock = _replace_once(
         lock_path.read_text(),
         (
-            rf'(^\[\[package\]\]\nname = "coval"\nversion = ")'
+            rf'(^\[\[package\]\]\nname = "{re.escape(name)}"\nversion = ")'
             rf"{re.escape(current)}" + r'("$)'
         ),
         rf"\g<1>{updated}\g<2>",
         lock_path,
     )
🤖 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 `@scripts/bump_version.py` around lines 42 - 56, Update the version-rewrite
logic around _replace_once so the Cargo.toml pattern targets only the [package]
manifest version rather than any matching version line. In the Cargo.lock
rewrite, derive the package name from the same Cargo.toml metadata used by
release_version() instead of hardcoding "coval", while preserving the existing
current-to-updated replacement behavior.
scripts/test_release_automation.py (1)

44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract _root into a module-level fixture instead of instantiating a second TestCase.

unittest.TestCase can be instantiated without a methodName in current Python, but borrowing _root from ReleaseVersionTests couples BumpVersionTests to another test class for a shared fixture. Move the temp directory factory out, or keep it as a @classmethod/@staticmethod on one test class and call it from BumpVersionTests.

🤖 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 `@scripts/test_release_automation.py` around lines 44 - 50, Extract the shared
_root temporary-directory fixture from ReleaseVersionTests into a module-level
helper, or expose it as a static/class method on a shared test utility. Update
BumpVersionTests.test_bumps_minor_version_in_both_manifests to use that helper
directly instead of instantiating ReleaseVersionTests, while preserving the
existing setup and cleanup behavior.
🤖 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/release-on-version-bump.yml:
- Around line 56-58: The release workflow’s automatic path currently fails when
release_version.py encounters a prerelease Cargo version. Update the
version-validation step and surrounding workflow logic to treat non-release
versions as a successful no-op for main pushes, while preserving the hard
failure behavior for workflow_dispatch runs; use the existing version step id
and event trigger conditions.

In @.github/workflows/release.yml:
- Around line 196-205: Update the Homebrew formula publishing logic around the
Formula/coval.rb change check to detect both tracked modifications and an
untracked formula, ensuring new formulas are added and committed instead of
reporting no changes. Replace the single git push in this workflow with
contention-safe retry handling that fetches and rebases or otherwise
incorporates concurrent origin/main updates before retrying the push, while
preserving the existing commit behavior.

---

Nitpick comments:
In @.github/workflows/api-parity-audit.yml:
- Line 26: Disable credential persistence on the actions/checkout@v6 step in
.github/workflows/api-parity-audit.yml lines 26-26 and .github/workflows/ci.yml
lines 19-19 by configuring persist-credentials to false in each step; no other
workflow changes are needed.

In @.github/workflows/ci.yml:
- Around line 38-39: Adjust the “Audit live API command coverage” step in the
check job so failures from the live api.coval.dev fetch do not block ordinary PR
and push CI runs, while preserving strict enforcement for the scheduled weekly
parity check. Use the workflow’s event context to apply the soft-fail or
short-circuit behavior only to non-scheduled runs.

In @.github/workflows/release.yml:
- Around line 32-44: Add a job-level permissions block to the validate job,
granting only contents: read. Keep the existing checkout, Python setup, and
release-version validation steps unchanged.
- Around line 186-194: Update the Homebrew tap clone and push flow around
tap_dir so HOMEBREW_TAP_TOKEN is supplied through Git’s HTTPS extraheader
authentication instead of embedded in the repository URL. Use the same
http.https://github.com/.extraheader configuration for both clone and push,
ensuring the token is not persisted in tap_dir/.git/config.

In `@scripts/bump_version.py`:
- Around line 42-56: Update the version-rewrite logic around _replace_once so
the Cargo.toml pattern targets only the [package] manifest version rather than
any matching version line. In the Cargo.lock rewrite, derive the package name
from the same Cargo.toml metadata used by release_version() instead of
hardcoding "coval", while preserving the existing current-to-updated replacement
behavior.

In `@scripts/release_version.py`:
- Line 14: Keep VERSION_RE in scripts/release_version.py as the single source of
truth. In scripts/render_homebrew_formula.py, remove the duplicate pattern and
import VERSION_RE from release_version using the existing package/script import
shim demonstrated in scripts/bump_version.py; no direct change is needed in
release_version.py.

In `@scripts/test_audit_api_coverage.py`:
- Around line 1-170: Add focused tests for _manifest_operations covering missing
reasons, invalid HTTP methods, and duplicate operations, asserting each
validation failure. Add audit() tests for overlapping manifest sections and for
correctly classifying operation gaps and extras, including the resulting
pass/fail decision used by CI and the weekly backstop. Reuse the existing
unittest and mock patterns without changing production behavior.

In `@scripts/test_release_automation.py`:
- Around line 44-50: Extract the shared _root temporary-directory fixture from
ReleaseVersionTests into a module-level helper, or expose it as a static/class
method on a shared test utility. Update
BumpVersionTests.test_bumps_minor_version_in_both_manifests to use that helper
directly instead of instantiating ReleaseVersionTests, while preserving the
existing setup and cleanup behavior.

In `@src/commands/traces.rs`:
- Around line 235-236: Add local validation in the trace command flow before
input_json::finish to reject cases where args.duration_ms_min exceeds
args.duration_ms_max, following the existing attribute-filter cap validation
pattern. Return a clear input error and preserve the current filter insertion
behavior for valid or partially specified ranges.
🪄 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: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 477a4563-d258-44fa-9303-da5eacd108a9

📥 Commits

Reviewing files that changed from the base of the PR and between 40905ee and ee921d8.

📒 Files selected for processing (22)
  • .github/workflows/api-parity-audit.yml
  • .github/workflows/ci.yml
  • .github/workflows/release-on-version-bump.yml
  • .github/workflows/release.yml
  • .gitignore
  • README.md
  • api-coverage.toml
  • scripts/audit_api_coverage.py
  • scripts/bump_version.py
  • scripts/release_version.py
  • scripts/render_homebrew_formula.py
  • scripts/requirements-audit.txt
  • scripts/test_audit_api_coverage.py
  • scripts/test_release_automation.py
  • src/agent_discovery.rs
  • src/cli.rs
  • src/client/mod.rs
  • src/client/models/mod.rs
  • src/client/models/trace.rs
  • src/commands/mod.rs
  • src/commands/traces.rs
  • tests/cli_tests.rs

Comment thread .github/workflows/release-on-version-bump.yml Outdated
Comment thread .github/workflows/release.yml Outdated
@callumreid callumreid changed the title [COVAL-4319] Automate weekly CLI API parity PRs and gated releases [COVAL-4319] Add weekly CLI API parity audit and gated releases Jul 30, 2026
@callumreid
callumreid force-pushed the callum/coval-4319-weekly-cli-parity branch from bb2306e to a382d3c Compare July 30, 2026 23:27
@callumreid callumreid changed the title [COVAL-4319] Add weekly CLI API parity audit and gated releases [COVAL-4319] Open weekly CLI API parity PRs and gate releases Jul 30, 2026

@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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
scripts/audit_api_coverage.py (1)

111-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Canonical collisions silently shrink the published operation set.

_canonical_operation maps every {...} segment to {id}, so two distinct published paths (or the same path duplicated across specs with different placeholder names) collapse onto one key and the later one overwrites the earlier. That understates published_operation_count and can hide a real gap while the snapshot check still passes. Fail loudly on a conflicting non-identical entry.

🛡️ Proposed guard
             for method in HTTP_METHODS & set(path_item):
                 canonical = _canonical_operation(method, path)
-                operations[canonical] = f"{method.upper()} {path}"
+                display = f"{method.upper()} {path}"
+                previous = operations.get(canonical)
+                if previous is not None and previous != display:
+                    raise RuntimeError(
+                        f"published operations collide on {canonical}: "
+                        f"{previous} and {display}"
+                    )
+                operations[canonical] = display
🤖 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 `@scripts/audit_api_coverage.py` around lines 111 - 122, The
_published_operations function silently overwrites entries when distinct
published paths produce the same canonical operation key. Before assigning to
operations[canonical], detect an existing non-identical operation value and
raise an error; preserve duplicate identical entries, while continuing to store
new canonical operations normally.
🧹 Nitpick comments (4)
.github/workflows/api-parity-audit.yml (1)

28-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add timeout-minutes to the scheduled job.

The audit performs live catalog/spec fetches with retries; without a job timeout a stalled run holds the weekly-api-parity-pr concurrency group (cancel-in-progress: false) until the 6-hour default expires.

⏱️ Proposed change
   refresh:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     steps:
🤖 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/api-parity-audit.yml around lines 28 - 30, Add a finite
timeout-minutes setting to the refresh job in the api-parity audit workflow,
choosing a duration appropriate for its retried live catalog/spec fetches and
shorter than the default six-hour limit. Keep the existing concurrency behavior
unchanged.
scripts/test_audit_api_coverage.py (1)

59-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the HTTPError retry branch.

Only the URLError path is exercised. The asymmetric policy in _fetch (retry on ≥500, re-raise immediately on 4xx) is the easiest part to regress silently — two small cases would pin it.

🤖 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 `@scripts/test_audit_api_coverage.py` around lines 59 - 78, Extend
test_retries_transient_network_failure coverage for _fetch with separate
HTTPError cases: verify a status at or above 500 is retried and eventually
succeeds, while a 4xx error is re-raised immediately without sleeping or
retrying. Reuse the existing build_opener and sleep mocks and preserve the
current URLError test.
scripts/requirements-audit.txt (1)

1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pins are valid; consider moving to the tip of the 0.15 line and committing an explicit Ruff config.

PyYAML 6.0.3 is the current latest, and ruff==0.15.9 resolves. Two notes: 0.15.20 is available on the same line, and Ruff 0.16 enables 413 rules by default, up from 59 — since CI runs ruff check scripts with defaults, an explicit [tool.ruff] rule selection will keep the eventual 0.16 bump from turning into a large unrelated diff.

🤖 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 `@scripts/requirements-audit.txt` around lines 1 - 2, Update the Ruff
dependency pin in requirements-audit.txt from 0.15.9 to the latest 0.15.x
release, 0.15.20. Add an explicit [tool.ruff] configuration with rule selection
matching the current intended `ruff check scripts` behavior, preventing a future
0.16 upgrade from enabling unrelated rules by default.
src/commands/traces.rs (1)

54-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the attribute-filter limit into a named constant.

The value 10 is duplicated across the help text and the validation check; extracting a const MAX_ATTRIBUTE_FILTERS: usize = 10; avoids drift if the limit changes.

♻️ Proposed refactor
+const MAX_ATTRIBUTE_FILTERS: usize = 10;
+
 #[derive(Args)]
 pub struct SearchArgs {
     ...
-    /// Attribute filter as KEY:OPERATOR[:VALUE]; repeat up to 10 times
+    /// Attribute filter as KEY:OPERATOR[:VALUE]; repeat up to MAX_ATTRIBUTE_FILTERS times
     #[arg(long = "attribute-filter")]
     attribute_filters: Vec<String>,
     if let Some(attribute_filters) = &request.filters.attribute_filters {
         anyhow::ensure!(
-            attribute_filters.len() <= 10,
-            "trace search accepts at most 10 attribute filters, got {}",
+            attribute_filters.len() <= MAX_ATTRIBUTE_FILTERS,
+            "trace search accepts at most {MAX_ATTRIBUTE_FILTERS} attribute filters, got {}",
             attribute_filters.len()
         );
     }

Also applies to: 257-263

🤖 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 `@src/commands/traces.rs` around lines 54 - 56, Extract the attribute-filter
limit into a named MAX_ATTRIBUTE_FILTERS constant with value 10, then reuse it
in both the attribute_filters help text and validation logic. Update the related
symbols in the command definition and validation path so the limit has a single
source of truth.
🤖 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/api-parity-audit.yml:
- Around line 18-21: Reduce the workflow permissions block by removing contents:
write and pull-requests: write; retain only issues: write, since the workflow’s
github-script steps require issue access while REGEN_PR_TOKEN handles repository
and pull-request operations.

In @.github/workflows/ci.yml:
- Around line 40-44: Update the “Audit live API command coverage” step in the CI
workflow so live third-party API failures or report drift do not block
pull-request checks; make the live audit informational/non-blocking while
retaining the weekly api-parity-audit workflow as the enforcement path.

In @.github/workflows/release-on-version-bump.yml:
- Around line 36-45: Update the workflow dispatch branch in the “Resolve the
CI-tested commit” step to validate that the selected ref is main before writing
GITHUB_SHA to the output. Reject non-main manual dispatches with a failing
command, while preserving the existing workflow_run SHA resolution and valid
main dispatch behavior.

---

Outside diff comments:
In `@scripts/audit_api_coverage.py`:
- Around line 111-122: The _published_operations function silently overwrites
entries when distinct published paths produce the same canonical operation key.
Before assigning to operations[canonical], detect an existing non-identical
operation value and raise an error; preserve duplicate identical entries, while
continuing to store new canonical operations normally.

---

Nitpick comments:
In @.github/workflows/api-parity-audit.yml:
- Around line 28-30: Add a finite timeout-minutes setting to the refresh job in
the api-parity audit workflow, choosing a duration appropriate for its retried
live catalog/spec fetches and shorter than the default six-hour limit. Keep the
existing concurrency behavior unchanged.

In `@scripts/requirements-audit.txt`:
- Around line 1-2: Update the Ruff dependency pin in requirements-audit.txt from
0.15.9 to the latest 0.15.x release, 0.15.20. Add an explicit [tool.ruff]
configuration with rule selection matching the current intended `ruff check
scripts` behavior, preventing a future 0.16 upgrade from enabling unrelated
rules by default.

In `@scripts/test_audit_api_coverage.py`:
- Around line 59-78: Extend test_retries_transient_network_failure coverage for
_fetch with separate HTTPError cases: verify a status at or above 500 is retried
and eventually succeeds, while a 4xx error is re-raised immediately without
sleeping or retrying. Reuse the existing build_opener and sleep mocks and
preserve the current URLError test.

In `@src/commands/traces.rs`:
- Around line 54-56: Extract the attribute-filter limit into a named
MAX_ATTRIBUTE_FILTERS constant with value 10, then reuse it in both the
attribute_filters help text and validation logic. Update the related symbols in
the command definition and validation path so the limit has a single source of
truth.
🪄 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: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: bca9bf4a-1e0c-44bb-9ed7-c9ebdcb8288e

📥 Commits

Reviewing files that changed from the base of the PR and between ee921d8 and cfb0eda.

📒 Files selected for processing (17)
  • .github/workflows/api-parity-audit.yml
  • .github/workflows/ci.yml
  • .github/workflows/release-on-version-bump.yml
  • .github/workflows/release.yml
  • .gitignore
  • README.md
  • api-coverage-report.md
  • api-coverage.toml
  • scripts/audit_api_coverage.py
  • scripts/bump_version.py
  • scripts/release_version.py
  • scripts/render_homebrew_formula.py
  • scripts/requirements-audit.txt
  • scripts/test_audit_api_coverage.py
  • scripts/test_release_automation.py
  • src/commands/traces.rs
  • tests/cli_tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

Comment thread .github/workflows/api-parity-audit.yml
Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/release-on-version-bump.yml
@callumreid
callumreid merged commit 1f33658 into main Jul 31, 2026
7 checks passed
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.

1 participant