chore: non-interactive release model download, fail-closed make targets, CI quality job, docs sync#56
Conversation
The model/embedder download scripts prompted with `read -p` under `set -euo pipefail`. On a cold CI runner without a TTY, `read` hits EOF and the script dies before xcodebuild, breaking the release DMG chain (release.yml -> make release-dmgs -> ensure-models -> download-model.sh). Gate the prompt on interactivity: when `CI=true` or stdin is not a TTY, skip the prompt and proceed. Both default models are public, so the download succeeds without auth; a gated model would fail at `hf download` with a clear hint to set HF_TOKEN. Interactive TTY behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test-e2e, test-e2e-real, test-sse, test-formatting, and test-all piped cargo test through tee without capturing PIPESTATUS, so a failing test run still exited 0 because the recipe's last command was an echo. Apply the same set -o pipefail + PIPESTATUS[0] + exit pattern already used in test/test-quick to each cargo test invocation in these targets. Also drop the `|| true` on the prettier check in `make check` so formatting failures fail the gate instead of being masked.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request updates the project's testing and build configuration, documentation, and download scripts. Key changes include updating the Makefile to use set -o pipefail and capture PIPESTATUS to ensure test failures are not masked by piping to tee, and removing || true from the Prettier formatting check to enforce formatting compliance. Additionally, the HuggingFace model and embedder download scripts were updated to support non-interactive execution in CI environments, the examples documentation was expanded to cover new demo files, and a semgrep warning bypass was added to a benchmark test helper. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ce parser Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace manual Option::filter implementation in non_empty_transcript with Option::filter, per clippy::manual_filter on stable 1.97. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR type: devex / release / docs
Problems addressed
From the same verified quality recon as #55:
scripts/download-model.sh(and its twindownload-embedder.sh) ranread -punderset -euo pipefail; on a cold CI runner with no TTY the read hits EOF and the release job exits beforexcodebuildeven starts.test-e2e,test-e2e-real,test-formatting,test-all,test-ssepipedcargo test | teewithoutPIPESTATUSand ended the recipe withecho— the target exited 0 even with failing tests.make checkadditionally masked prettier failures with|| true.qualityjob inrust.ymlwas commented out; onlycargo fmtgated PRs.examples/README.mddocumented two non-existent examples, omitted all 8 real ones and claimed Rust 1.70+ (workspace needs 1.85+/edition 2024);NOTICEused non-brandCodeScribecasing;CHANGELOG.mdwas missing entries for merged PRs feat(settings): API key liveness probe in Keys panel #50–fix(onboarding): open Settings via SwiftUI openSettings action #54.Fixes
scripts/download-model.sh,scripts/download-embedder.sh:CI=trueor no TTY → no prompt; clear non-interactive message (withHF_TOKENhint) then normal download. Interactive TTY behavior unchanged.release.ymlneeds no token: both default models verified public (gated: False).Makefile: the exactset -o pipefail+PIPESTATUS[0]pattern already used bytest/test-quickapplied to all five targets (bothcargo testinvocations insidetest-sse); prettier|| trueremoved fromcheck..github/workflows/rust.yml:qualityjob (clippy-D warnings+cargo test --workspacewithCODESCRIBE_DISABLE_KEYCHAIN=1) re-enabled on the same self-hosted/toolchain pins as the existingformatjob;#[ignore]tests (real API/audio/models) stay excluded, documented in a workflow comment; nocontinue-on-error.examples/README.md,NOTICE,CHANGELOG.md: synced to reality (all 8 examples documented and verified compiling viacargo check --examples; brand casing fixed; changelog caught up).core/pipeline/streaming/tests.rs: one-line justifiednosemgrepsuppression.Test plan (commands actually run)
Smoke notes
qualityCI run on this PR is the live verification of the re-enabled job — if it sits inqueued, that is runner-group config (allows_public_repositories), not the workflow.make release-dmgson a cold checkout without models now either downloads or fails fast with a clear message — no interactive hang.Out of scope / backlog
Model source default, bench reproducibility (
bench-stt.shprivate-corpus preference), remaining P3 docs items — recorded in the recon backlog.🤖 Generated with Claude Code