Repository navigation
ci: add release size audit and native backend checks - #55
Merged
Merged
Conversation
Reviewer's GuideThis PR adds CI-enforced release size and performance auditing through frozen production binaries, paired native benchmarks, and a new audit tool, while restructuring backend feature selection and validating Linux native dependency graphs across supported windowing variants. Sequence diagram for the release size and performance auditsequenceDiagram
participant CI as Weekly release workflow
participant Cargo
participant Audit as chitin-release-audit
participant Criterion
participant Artifacts as Frozen executables
CI->>Cargo: cargo build -p chitin-desktop --release --locked
CI->>Cargo: just build-desktop-release
CI->>Artifacts: Freeze production executables
CI->>Cargo: cargo test -p chitin-release-audit --locked
CI->>Cargo: cargo test -p chitin-bio -p chitin-molecule-renderer --lib --profile release-size --locked
CI->>Audit: just benchmark-release-profiles
Audit->>Cargo: cargo bench --no-run --profile release
Audit->>Cargo: cargo bench --no-run --profile release-size
Audit->>Criterion: Run baseline/optimized/optimized/baseline phases
Criterion-->>Audit: Saved medians and raw samples
CI->>Audit: cargo run -- --paired
Audit->>Artifacts: Read executable sizes
Audit->>Criterion: Read paired medians
Audit-->>CI: comparison.json and budget result
Sequence diagram for the native offscreen frame benchmarksequenceDiagram
participant Criterion
participant Benchmark as native_frame
participant Parser as PdbParser
participant Adapter as Native wgpu adapter
participant Renderer as MoleculeRenderer
participant GPU as Device and queue
Criterion->>Benchmark: native_structure_parse_512_atoms
Benchmark->>Parser: parse_bytes
Parser-->>Benchmark: Parsed structure
Criterion->>Benchmark: native_molecule_frame_512_atoms
Benchmark->>Adapter: request_adapter
Adapter-->>Benchmark: Native adapter
Benchmark->>GPU: request_device
Benchmark->>Renderer: MoleculeRenderer::new
loop Each measured frame
Benchmark->>Renderer: render
Renderer->>GPU: Submit render commands
Benchmark->>GPU: poll
GPU-->>Benchmark: Completed submission
end
Flow diagram for Linux native backend verificationflowchart TD
Start[Linux host check]
Variants[Check default, wayland, and x11 variants]
Tree[cargo tree feature inspection]
Vulkan{Vulkan enabled only}
Window{Selected window backend matches variant}
Wayland{Wayland variant excludes X11 pointer polling}
Compile[cargo check]
Pass[Backend verification passes]
Fail[Fail with dependency or feature diagnostic]
Start --> Variants
Variants --> Tree
Tree --> Vulkan
Vulkan -->|no| Fail
Vulkan -->|yes| Window
Window -->|no| Fail
Window -->|yes| Wayland
Wayland -->|invalid| Fail
Wayland -->|valid or not applicable| Compile
Compile -->|failure| Fail
Compile -->|success| Pass
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/weekly-release-build.yml" line_range="99" />
<code_context>
+ target/release-frozen/optimized/${{ matrix.desktop_executable }} \
+ | tee target/release-audit/executable-bytes.txt
+
+ - name: Check release audit tests
+ run: cargo test -p chitin-release-audit --locked
+
</code_context>
<issue_to_address>
**Backend regressions pass CI**
When the workflow runs after a native backend feature change, `weekly-release-build.yml` and the regular CI workflow never invoke `tools/check-native-backends.sh`, so its feature-graph assertions and Linux configuration checks do not run; backend regressions pass CI unnoticed and an unexpected or unusable Linux build can ship.
Invoke `tools/check-native-backends.sh` from the weekly release workflow and regular CI.
Also at `.github/workflows/weekly-release-build.yml:102`, `tools/check-native-backends.sh:1`, `tools/check-native-backends.sh:53`.
</issue_to_address>
### Comment 2
<location path="tools/chitin-release-audit/src/measure.rs" line_range="103-105" />
<code_context>
+/// Success only after all four phases of all workloads finish. Their raw samples
+/// and executable paths remain available for independent inspection.
+pub(super) fn run(root: &Path) -> Result<()> {
+ fs::create_dir_all(root)?;
+ ensure!(
+ fs::read_dir(root)?.next().is_none(),
+ "measurement directory must be empty: {}",
+ root.display()
</code_context>
<issue_to_address>
**Concurrent audits mix measurements**
When two `measure` invocations use the same root concurrently, both `measure` processes can observe the same empty root, then run Criterion with identical phase labels and write into the same result directories. Their outputs can interleave or overwrite one another, so the later comparison can pass using mixed measurements.
Acquire an exclusive lock or atomically claim the measurement root before checking or writing it, and fail the second invocation if the claim already exists.
</issue_to_address>
### Comment 3
<location path="tools/chitin-release-audit/src/main.rs" line_range="224-225" />
<code_context>
+ let timings = compare_timings(&read(Path::new(root), baseline)?, &read(Path::new(root), candidate)?)?;
+ // A partially executed benchmark script must not produce a passing release.
+ for required in ["native_structure_parse_512_atoms", "native_molecule_frame_512_atoms"] {
+ ensure!(
+ timings.iter().any(|timing| timing.benchmark == required),
+ "missing benchmark {required}"
+ );
+ }
+ for prefix in [
+ "sequence_columns/visible_columns_after/",
+ "sequence_box_hit/row_interval_after/",
+ "sequence_wrapped_row_lookup_",
+ "kit_tree_",
+ "molecular_surface",
+ ] {
+ ensure!(
+ timings.iter().any(|timing| timing.benchmark.starts_with(prefix)),
+ "missing benchmark family {prefix}"
+ );
</code_context>
<issue_to_address>
**Missing benchmark cases pass the audit**
When an individual benchmark case is removed or omitted from both profile runs while another case in its family remains, the prefix check accepts any single result in a benchmark family, while `compare_timings` only requires both profiles to have matching sets. Removing or disabling a case such as the 100,000-residue workload from both profiles therefore leaves that workload ungated while the audit can pass.
Validate the full expected benchmark-name set, including every scale and variant, rather than only requiring one result per prefix.
</issue_to_address>Sourcery assessment
Approval pending. 3 findings to address first.
Blocking findings: .github/workflows/weekly-release-build.yml:99, tools/chitin-release-audit/src/measure.rs:105, tools/chitin-release-audit/src/main.rs:225
| target/release-frozen/optimized/${{ matrix.desktop_executable }} \ | ||
| | tee target/release-audit/executable-bytes.txt | ||
|
|
||
| - name: Check release audit tests |
There was a problem hiding this comment.
🟡 Medium · Backend regressions pass CI
When the workflow runs after a native backend feature change, weekly-release-build.yml and the regular CI workflow never invoke tools/check-native-backends.sh, so its feature-graph assertions and Linux configuration checks do not run; backend regressions pass CI unnoticed and an unexpected or unusable Linux build can ship.
Invoke tools/check-native-backends.sh from the weekly release workflow and regular CI.
Also at .github/workflows/weekly-release-build.yml:102, tools/check-native-backends.sh:1, tools/check-native-backends.sh:53.
Prompt for AI agents
In `.github/workflows/weekly-release-build.yml` at line 99:
**Backend regressions pass CI**
When the workflow runs after a native backend feature change, `weekly-release-build.yml` and the regular CI workflow never invoke `tools/check-native-backends.sh`, so its feature-graph assertions and Linux configuration checks do not run; backend regressions pass CI unnoticed and an unexpected or unusable Linux build can ship.
Invoke `tools/check-native-backends.sh` from the weekly release workflow and regular CI.
Also at `.github/workflows/weekly-release-build.yml:102`, `tools/check-native-backends.sh:1`, `tools/check-native-backends.sh:53`.
Comment on lines
+103
to
+105
| fs::create_dir_all(root)?; | ||
| ensure!( | ||
| fs::read_dir(root)?.next().is_none(), |
There was a problem hiding this comment.
🟡 Medium · Concurrent audits mix measurements
When two measure invocations use the same root concurrently, both measure processes can observe the same empty root, then run Criterion with identical phase labels and write into the same result directories. Their outputs can interleave or overwrite one another, so the later comparison can pass using mixed measurements.
Acquire an exclusive lock or atomically claim the measurement root before checking or writing it, and fail the second invocation if the claim already exists.
Prompt for AI agents
In `tools/chitin-release-audit/src/measure.rs` at lines 103-105:
**Concurrent audits mix measurements**
When two `measure` invocations use the same root concurrently, both `measure` processes can observe the same empty root, then run Criterion with identical phase labels and write into the same result directories. Their outputs can interleave or overwrite one another, so the later comparison can pass using mixed measurements.
Acquire an exclusive lock or atomically claim the measurement root before checking or writing it, and fail the second invocation if the claim already exists.
Comment on lines
+224
to
+225
| ensure!( | ||
| timings.iter().any(|timing| timing.benchmark.starts_with(prefix)), |
There was a problem hiding this comment.
🟡 Medium · Missing benchmark cases pass the audit
When an individual benchmark case is removed or omitted from both profile runs while another case in its family remains, the prefix check accepts any single result in a benchmark family, while compare_timings only requires both profiles to have matching sets. Removing or disabling a case such as the 100,000-residue workload from both profiles therefore leaves that workload ungated while the audit can pass.
Validate the full expected benchmark-name set, including every scale and variant, rather than only requiring one result per prefix.
Prompt for AI agents
In `tools/chitin-release-audit/src/main.rs` at lines 224-225:
**Missing benchmark cases pass the audit**
When an individual benchmark case is removed or omitted from both profile runs while another case in its family remains, the prefix check accepts any single result in a benchmark family, while `compare_timings` only requires both profiles to have matching sets. Removing or disabling a case such as the 100,000-residue workload from both profiles therefore leaves that workload ungated while the audit can pass.
Validate the full expected benchmark-name set, including every scale and variant, rather than only requiring one result per prefix.
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
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
Scope
Merge 3 commits from
ci/release-size-20261027intofeat/chemdraw-sketcher(branch is ahead by 3 commits and behind by 0).Validation
CI and checks have not been independently executed as part of creating this PR.
Summary by Sourcery
Add release size and performance auditing with native backend verification to the release build pipeline.
New Features:
Enhancements:
Build:
CI:
Tests:
Chores: