Pass Polonius flags through shared Rust setup - #80
Conversation
|
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:
Summary
WalkthroughUpdate CI, coverage, and release templates to use pinned shared actions and conditional Rust flags. Move release ChangesRust workflow migration
Sequence Diagram(s)sequenceDiagram
participant WorkflowTemplates as Workflow templates
participant SetupRust as setup-rust
participant BuildSteps as Rust and coverage steps
WorkflowTemplates->>SetupRust: Pass -Zpolonius=next or -D warnings
SetupRust->>BuildSteps: Provide base RUSTFLAGS
WorkflowTemplates->>BuildSteps: Add lld for coverage
BuildSteps-->>WorkflowTemplates: Log selected compiler configuration
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (18 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuidePass Polonius configuration through the shared setup-rust action for CI, coverage, and release workflows, and update the Polonius tooling contracts/tests to assert the new behavior. Flow diagram for Polonius rustflags passthrough in CI/coverage/release workflowsflowchart TD
start([Workflow start])
start --> enable_polonius
enable_polonius{enable_polonius}
enable_polonius -->|true| setup_rust_polonius
enable_polonius -->|false| setup_rust_warnings
setup_rust_polonius["Setup Rust (setup-rust)\nwith rustflags = -Zpolonius=next"]
setup_rust_warnings["Setup Rust (setup-rust)\nwith rustflags = -D warnings"]
setup_rust_polonius --> coverage_polonius
setup_rust_polonius --> release_polonius
setup_rust_warnings --> coverage_warnings
setup_rust_warnings --> release_warnings
coverage_polonius["Test and Measure Coverage (generate-coverage)\nRUSTFLAGS includes -Zpolonius=next and -C link-arg=-fuse-ld=lld"]
coverage_warnings["Test and Measure Coverage (generate-coverage)\nRUSTFLAGS includes -D warnings and -C link-arg=-fuse-ld=lld"]
release_polonius["Build release binary (cross)\nuses nightly toolchain with -Zpolonius=next from setup-rust"]
release_warnings["Build release binary (cross)\nuses stable toolchain with -D warnings from setup-rust"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@template/.github/workflows/ci.yml.jinja`:
- Around line 26-28: Preserve -D warnings in both coverage workflows when
enable_polonius is disabled by adding it to the linker-only RUSTFLAGS branches
in template/.github/workflows/ci.yml.jinja (lines 26-28) and
template/.github/workflows/coverage-main.yml.jinja (lines 35-37). Update
tests/helpers/tooling_contracts/polonius.py to assert this disabled-coverage
value.
In `@tests/helpers/tooling_contracts/polonius.py`:
- Line 54: Update the assertions in the contract checks around the rustflags
assertion and the release assertions at lines 150-157 to include diagnostic
messages instead of bare asserts. Capture each actual value, then include the
workflow or job name, expected value, and actual value in the assertion message,
following the existing setup-rust message style and using assert …, "message".
🪄 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: fb6a398f-df42-48c9-91e9-3b449cb3a98e
📒 Files selected for processing (5)
template/.github/workflows/ci.yml.jinjatemplate/.github/workflows/coverage-main.yml.jinjatemplate/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinjatests/helpers/tooling_contracts/polonius.pytests/test_template/__snapshots__/test_snapshots.ambr
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
|
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/users-guide.md`:
- Around line 51-56: Qualify the release workflow description as applying only
when flavour is app, clarifying that library renders do not include release.yml.
Update both docs/users-guide.md lines 51-56 and
template/docs/developers-guide.md.jinja lines 47-51 consistently; no other
workflow descriptions require changes.
In `@tests/helpers/tooling_contracts/polonius.py`:
- Around line 32-44: Update _setup_rust_step to match only the shared setup-rust
action using the required leynos/shared-actions/.github/actions/setup-rust@ path
followed by exactly 40 hexadecimal characters before asserting the single match.
Replace the current broad substring check while preserving the existing
uniqueness assertion and return behavior.
🪄 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: cd9a7014-1c7a-480c-9590-470cf8427163
📒 Files selected for processing (7)
docs/users-guide.mdtemplate/.github/workflows/ci.yml.jinjatemplate/.github/workflows/coverage-main.yml.jinjatemplate/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinjatemplate/docs/developers-guide.md.jinjatests/helpers/tooling_contracts/polonius.pytests/test_template/__snapshots__/test_snapshots.ambr
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/developers-guide.md`:
- Line 31: In the sentence describing coverage’s explicit RUSTFLAGS override,
remove the comma before “because” so the essential explanatory clause remains
attached to the main clause. Preserve the existing en-GB wording and required
grammar triage tag.
In `@template/docs/developers-guide.md.jinja`:
- Around line 47-53: Update the “Workflow pins and Dependabot” section in the
generated guide to document the narrow RUSTFLAGS_PASSTHROUGH_REVISION exception:
assert the first capability-bearing revision only when no independent capability
probe exists, clearly define that boundary, and remove the literal revision
assertion once such a probe is available. Keep the existing general guidance
against exact SHA assertions outside this exception.
🪄 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: 77a02194-a24a-4964-b425-cf9132a1ceab
📒 Files selected for processing (5)
docs/developers-guide.mddocs/execplans/rust-project-enhancements.mddocs/users-guide.mdtemplate/docs/developers-guide.md.jinjatests/helpers/tooling_contracts/polonius.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32231da1d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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 `@template/docs/developers-guide.md.jinja`:
- Around line 88-94: The workflow-pin policy in
template/docs/developers-guide.md.jinja must apply the same conditional
RUSTFLAGS_PASSTHROUGH_REVISION exception to the prohibition around lines
117–119: allow the literal revision only until an independent capability probe
confirms setup-rust supports rustflags, then require and document the first
capable revision and remove the assertion once probing exists. Update the
existing policy near the RUSTFLAGS_PASSTHROUGH_REVISION guidance; make no change
to docs/developers-guide.md at lines 143–148, which has no corresponding
prohibition.
🪄 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: 957e187e-5972-4233-b111-0ad35d30a17e
📒 Files selected for processing (11)
docs/developers-guide.mddocs/execplans/rust-project-enhancements.mddocs/users-guide.mdtemplate/.github/workflows/ci.yml.jinjatemplate/.github/workflows/coverage-main.yml.jinjatemplate/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinjatemplate/docs/developers-guide.md.jinjatests/helpers/tooling_contracts/polonius.pytests/test_github_actions_integration.pytests/test_template/__snapshots__/test_snapshots.ambrtests/test_template/test_tooling_contracts.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (2 warnings)
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata and checks =='
gh pr view 80 --json url,headRefName,statusCheckRollup,files \
--jq '{url, headRefName, checks: [.statusCheckRollup[] | {name, conclusion, status}], files: [.files[].path]}'
echo
echo '== Relevant file outlines =='
ast-grep outline tests/helpers/tooling_contracts/polonius.py --items all || true
ast-grep outline tests/test_template/test_tooling_contracts.py --items all --match 'polonius|Polonius' || true
ast-grep outline tests/test_github_actions_integration.py --items all --match 'RUST_SETUP_LOG_STEP|rust|Rust' || true
echo
echo '== Rust flags, logging, and contract-test evidence =='
rg -n -C 3 \
'setup-rust|rustflags|RUSTFLAGS|Log Rust compiler configuration|test_polonius_contract|RUST_SETUP_LOG_STEP' \
template/.github/workflows \
tests/helpers/tooling_contracts/polonius.py \
tests/test_template/test_tooling_contracts.py \
tests/test_github_actions_integration.py || true
echo
echo '== Recent validation evidence in PR comments =='
gh pr view 80 --comments --json comments \
--jq '.comments[] | select(.body | test("test|Test|pytest|act|validation|Validation"; "i")) | {author: .author.login, body: .body}' || trueLength of output: 62959
No further implementation work or GitHub issue is required for these two warnings. The current pre-merge report marks both checks as passed. You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/test_github_actions_integration.py`:
- Around line 59-60: Update the workflow path in the test invocation near
assert_ci_exercised_expected_steps to run the act-validation workflow that emits
ACT_VALIDATION_STEP with make test WITH_ACT=1, preserving the existing
saw_test_step and saw_rust assertions; alternatively, revise those assertions to
match steps produced by ci.yml.
- Around line 171-177: Remove the unused project parameter from
assert_act_result and update every call site to stop passing the CopierProject
argument, preserving the function’s remaining arguments and behavior.
🪄 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: eb31e1ed-2331-467e-8788-35dddeba8c8a
📒 Files selected for processing (8)
template/.github/workflows/ci.yml.jinjatemplate/.github/workflows/coverage-main.yml.jinjatemplate/.github/workflows/{% if flavour == 'app' %}release.yml{% endif %}.jinjatemplate/docs/developers-guide.md.jinjatests/helpers/tooling_contracts/polonius.pytests/test_github_actions_integration.pytests/test_template/__snapshots__/test_snapshots.ambrtests/test_template/test_tooling_contracts.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
Forward the generated project's selected `RUSTFLAGS` through the shared `setup-rust` action so toolchain setup no longer shadows Cargo's Polonius configuration. Keep coverage-specific linker flags at the coverage action boundary and remove the redundant release build override. Pin affected shared-action uses to the revision that provides the new passthrough and extend rendered-workflow contracts and snapshots.
Keep `-D warnings` when disabled Polonius workflows override `RUSTFLAGS` with coverage linker configuration. Assert the complete coverage flag value and add expected-versus-actual diagnostics to the setup and release workflow contracts.
Record that generated workflows depend on the shared `setup-rust` `rustflags` input and must preserve the passthrough when updating pins. Explain how coverage combines the selected base flags with its linker override, and keep literal shared-action SHAs out of contract tests.
Show how the Polonius selection controls setup, coverage, and release flags, including the coverage linker override and release toolchain. Provide a screen-reader description and an accessible figure caption.
Restrict setup-step matching to the pinned shared action and verify the passthrough capability revision across affected workflow references. Cover enabled and disabled coverage log output, qualify release guidance as application-only, and record the completed boundary in developer documentation and the living ExecPlan.
Keep the causal coverage explanation grammatically attached and document the narrow revision assertion permitted until an independent capability probe is available.
Log the selected compiler and bounded base flags after Rust setup across CI, coverage, and release workflows. Add direct regression tests for the passthrough contracts and require the existing `act` harness to observe the diagnostic.
Read the exported `RUSTFLAGS` value after setup so diagnostics and the integration harness detect passthrough failures. Restrict the capability revision contract to `setup-rust`, allowing unrelated shared actions to advance independently.
Apply the documented `RUSTFLAGS_PASSTHROUGH_REVISION` exception to the generated guide's closing prohibition until an independent capability probe is available.
Target the generated CI workflow explicitly in the `act` harness and verify its effective base flags for both warning-only and Polonius renders.
Detect the coverage composite's emitted Cargo commands instead of the separate act-validation step, and remove the unused rendered-project argument from result validation.
Apply the repository formatter after Weave combined the branch and main contract helpers.
77f6379 to
ff3b8c5
Compare
Summary
This branch updates generated Polonius-aware workflows to use the shared
setup-rustaction'srustflagspassthrough. This prevents toolchain setupfrom replacing the generated project's required
-Zpolonius=nextflag whilepreserving the historical
-D warningsdefault when Polonius is disabled.The affected shared-action references are pinned to the revision that adds the
passthrough. Coverage steps retain their explicit linker flags, while release
builds no longer repeat
RUSTFLAGSafter setup has exported it.Review walkthrough
setup-rustwhile coverage-specificlldflags remain at the coverage boundary.Validation
make test: passed, 89 tests passed and 1 skipped.make spelling: passed.git diff --check: passed.Summary by Sourcery
Pass Polonius-related Rust flags through the shared setup-rust action across CI, coverage, and release workflows.
Enhancements:
Tests:
References