fix(ops): make recording exporters honor --help without writing artifacts - #52
fix(ops): make recording exporters honor --help without writing artifacts#52nish3451 wants to merge 4 commits into
Conversation
…acts The recording queue, cockpit, teleprompter, and rehearsal-check exporters ignored --help/-h and generated their artifacts anyway. Each now handles --help/-h before doing any work: it prints usage and exits 0 without writing or overwriting any recording artifact (and without spawning draft-prospect-message in the cockpit exporter). Extend test-active-operator-surfaces.mjs with a --help surface for all four recording exporters (exit 0 + usage + no artifact overwrite).
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe four recording exporters now support ChangesRecording exporter help
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@scripts/test-active-operator-surfaces.mjs`:
- Around line 104-112: Extend the recordingHelpSurface test loop to execute each
script with both --help and -h. Apply the existing exit-status, usage-output,
and artifact-preservation assertions independently for each flag, ensuring
sentinel files are recreated and cleaned up for every invocation.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84532485-3864-4c0a-ae50-07acd340d600
📒 Files selected for processing (5)
scripts/export-recording-cockpit.mjsscripts/export-recording-queue.mjsscripts/export-recording-rehearsal-check.mjsscripts/export-recording-teleprompter.mjsscripts/test-active-operator-surfaces.mjs
| for (const [name, artifactPaths] of recordingHelpSurface) { | ||
| for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n") | ||
| const helped = run([`scripts/${name}`, "--help"]) | ||
| eq(helped.status, 0, `${name} --help must exit 0: ${helped.stderr || helped.stdout}`) | ||
| mat(helped.stdout, /Usage:/, `${name} --help must print usage`) | ||
| for (const path of artifactPaths) { | ||
| deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} --help must not overwrite ${path}`) | ||
| unlinkSync(join(T, path)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the -h path as well as --help.
This loop invokes each exporter only with --help. Run the same status, usage, and artifact-preservation assertions for both supported flags.
Suggested test adjustment
for (const [name, artifactPaths] of recordingHelpSurface) {
- for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n")
- const helped = run([`scripts/${name}`, "--help"])
- eq(helped.status, 0, `${name} --help must exit 0: ${helped.stderr || helped.stdout}`)
- mat(helped.stdout, /Usage:/, `${name} --help must print usage`)
- for (const path of artifactPaths) {
- deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} --help must not overwrite ${path}`)
- unlinkSync(join(T, path))
+ for (const flag of ["--help", "-h"]) {
+ for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n")
+ const helped = run([`scripts/${name}`, flag])
+ eq(helped.status, 0, `${name} ${flag} must exit 0: ${helped.stderr || helped.stdout}`)
+ mat(helped.stdout, /Usage:/, `${name} ${flag} must print usage`)
+ for (const path of artifactPaths) {
+ deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} ${flag} must not overwrite ${path}`)
+ unlinkSync(join(T, path))
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const [name, artifactPaths] of recordingHelpSurface) { | |
| for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n") | |
| const helped = run([`scripts/${name}`, "--help"]) | |
| eq(helped.status, 0, `${name} --help must exit 0: ${helped.stderr || helped.stdout}`) | |
| mat(helped.stdout, /Usage:/, `${name} --help must print usage`) | |
| for (const path of artifactPaths) { | |
| deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} --help must not overwrite ${path}`) | |
| unlinkSync(join(T, path)) | |
| } | |
| for (const [name, artifactPaths] of recordingHelpSurface) { | |
| for (const flag of ["--help", "-h"]) { | |
| for (const path of artifactPaths) writeFileSync(join(T, path), "help sentinel\n") | |
| const helped = run([`scripts/${name}`, flag]) | |
| eq(helped.status, 0, `${name} ${flag} must exit 0: ${helped.stderr || helped.stdout}`) | |
| mat(helped.stdout, /Usage:/, `${name} ${flag} must print usage`) | |
| for (const path of artifactPaths) { | |
| deq(readFileSync(join(T, path), "utf8"), "help sentinel\n", `${name} ${flag} must not overwrite ${path}`) | |
| unlinkSync(join(T, 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/test-active-operator-surfaces.mjs` around lines 104 - 112, Extend the
recordingHelpSurface test loop to execute each script with both --help and -h.
Apply the existing exit-status, usage-output, and artifact-preservation
assertions independently for each flag, ensuring sentinel files are recreated
and cleaned up for every invocation.
|
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.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
The fleet keeps dispatching the same finding to multiple lanes, producing duplicate same-fix PR pairs: #36/#44 and #55/#56 are byte-identical or near-identical patches of the operator export --help fix, #39/#49 the offername article fix, #40/#52 the recording exporter --help fix, plus #38/#48/#51, #30/#61/#72, #45/#64, #42/#60, #43/#54 and #46/#74. Add a PR Duplicate Guard: - scripts/check-pr-duplicates.mjs compares a PR's diff against every other open PR (shared changed-file coverage >= 0.8 and patch similarity >= 0.5). Calibrated on all 79 open PRs on 2026-08-11: every pair above the thresholds was a genuine duplicate-cluster member, zero false positives. - .github/workflows/pr-duplicate-guard.yml runs it on every PR event and posts one marker comment naming the duplicate(s) and the canonical PR; the check fails loudly when a duplicate is found. Informational, not required. - scripts/test-pr-duplicates.mjs covers parsing, similarity, detection, and comment upsert with an injected API; wired into npm ci and npm test.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Correction to my closing comment above: the keeper for this cluster is #160, not #56. I had wrongly concluded that This PR stays closed either way — its content is superseded by what is already on |
…main by PR #80 (duplicate guard) (#234) The duplicate-PR guard (PR #80, merged 2026-08-19) is now live on main, preventing the same-fix duplication pattern at the CI level. The named duplicate pairs (#36/#44, #39/#49, #40/#52) are functionally superseded: their underlying fixes are on main via PRs #135, #145, and #178. The prior 2026-08-15 lane run had concluded the guard existed but was stuck unmerged; on 2026-08-19 it landed and the item is resolved at the root-cause level. Verification-only run (no product or test files touched): - PR #80 source commit 2091c7a and merge commit 0a9909b are ancestors of origin/main - scripts/test-pr-duplicates.mjs → 'test-pr-duplicates: ok' (exit 0) - Superseding PRs #135 (d4f3ef4), #145 (fc44b42), #178 (77f6922) all on main - Guard workflow runs on pull_request events; not a required status, so existing work never blocks Co-authored-by: minimax-vps <minimax-vps@nish3451.dev>
1 participant
What
The recording exporters (queue / cockpit / teleprompter / rehearsal-check) ignored
--help/-hand generated their artifacts anyway, overwriting files likeprospects/recording-cockpit.html.Fix
Each of the four exporters now handles
--help/-hbefore doing any work: it prints usage and exits 0 without writing or overwriting any recording artifact (and without spawningdraft-prospect-messagein the cockpit exporter).scripts/export-recording-queue.mjsscripts/export-recording-cockpit.mjsscripts/export-recording-teleprompter.mjsscripts/export-recording-rehearsal-check.mjsTests
Extends
scripts/test-active-operator-surfaces.mjswith a--helpsurface for all four recording exporters: exit 0, usage printed, sentinel artifacts not overwritten.node scripts/test-active-operator-surfaces.mjspassesnpm testpasses (all suites, 0 failures)Summary by CodeRabbit
New Features
--helpand-hoptions to recording export commands.Tests