fix(ops): stop tracked 11/10 proof-run brief from silently reporting a zero pipeline - #45
fix(ops): stop tracked 11/10 proof-run brief from silently reporting a zero pipeline#45nish3451 wants to merge 3 commits into
Conversation
…a zero pipeline Running market:proof-run in a checkout with no private prospect state exited 0 with "created" and overwrote the git-tracked 11-10-proof-run.md with an all-zero pipeline (0/5, 0/40) plus a fake placeholder approved row in prospects/loom-links.txt. The exporter was also the only tracked-artifact generator not anchored to the service root, so a mixed-root invocation could silently report a wrong zero direction gate while its sub-processes read the real root. - Anchor every read and write to the service root (SERVICE_REPO_ROOT or the invocation directory), matching the other operator surfaces, and run sub-processes with the service root as cwd. - Refuse (exit 1, loud stderr, no writes) when the target is the tracked default brief and the service root holds no prospects/ state, so an unavailable pipeline is never mistaken for an empty one. Explicit --output= under runs/ keeps generating zero-state private reports and now warns on stderr when the prospect root is absent. - Keep the loom sheet merge dedupe stable by emitting service-root relative row paths and resolving sheet row paths against the service root for the gate counters. - Regenerate the tracked brief from a clean tree at the fixed clock date so the byte-identical operator-surface gate stays green, and extend the direction-proof-gate test with refusal and CWD-anchoring coverage.
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.
📝 WalkthroughWalkthroughThe proof exporter now resolves paths from ChangesProof export and regeneration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Exporter as export-market-proof-run.mjs
participant ServiceRoot as serviceRoot
participant Filesystem
Caller->>Exporter: Invoke export
Exporter->>ServiceRoot: Resolve paths and working directory
Exporter->>Filesystem: Check prospect pipeline state
alt Prospect state unavailable and output is tracked
Exporter->>Filesystem: Preserve tracked proof brief
else Output is private or prospect state exists
Exporter->>Filesystem: Write proof and Loom-link outputs
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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/export-market-proof-run.mjs`:
- Around line 41-46: Replace the existsSync(prospectRoot) checks in the
tracked-regeneration refusal and warning branches with a shared predicate that
confirms at least one outbound prospect folder contains pipeline.json. Reuse
this predicate for both branches, preserving the existing refusal and warning
behavior. Extend the relevant test after the private-output case to run the
default output and assert it still refuses.
🪄 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: bcd01358-24bf-4add-b1fd-cf52e0fe0cb1
📒 Files selected for processing (3)
growth-brain/ops/11-10-proof-run.mdscripts/export-market-proof-run.mjsscripts/test-direction-proof-gate.mjs
| if (regeneratesTrackedBrief && !existsSync(prospectRoot)) { | ||
| console.error(`Refusing to regenerate the tracked 11/10 proof-run brief with a zero pipeline: no outbound prospect state found at ${prospectRoot}. Run this command from the service root that holds prospects/, or set SERVICE_REPO_ROOT to it, or pass an explicit --output= under runs/ for a private zero-state report.`); | ||
| process.exit(1); | ||
| } | ||
| if (!existsSync(prospectRoot)) { | ||
| console.warn(`Warning: no outbound prospect state found at ${prospectRoot}; pipeline counts in ${outputPath} will be zero.`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Check actual prospect pipeline state before allowing tracked regeneration.
existsSync(prospectRoot) is true for an empty prospects/ directory. It is also true after an explicit private report creates prospects/loom-links.txt at Line 371. A private zero-state run followed by a default run can therefore overwrite the tracked brief with zero counts.
Require at least one outbound prospect folder with pipeline.json. Use the same predicate for the refusal and warning. Extend the test after Line 147 to run the default output after the private output and assert that it still refuses.
Proposed fix
const regeneratesTrackedBrief = resolve(resolvedOutputPath) === resolve(trackedBriefPath);
-if (regeneratesTrackedBrief && !existsSync(prospectRoot)) {
+const hasProspectPipelineState = existsSync(prospectRoot)
+ && listFolders(prospectRoot).some((path) => existsSync(join(path, "pipeline.json")));
+
+if (regeneratesTrackedBrief && !hasProspectPipelineState) {
console.error(`Refusing to regenerate the tracked 11/10 proof-run brief with a zero pipeline: no outbound prospect state found at ${prospectRoot}. Run this command from the service root that holds prospects/, or set SERVICE_REPO_ROOT to it, or pass an explicit --output= under runs/ for a private zero-state report.`);
process.exit(1);
}
-if (!existsSync(prospectRoot)) {
+if (!hasProspectPipelineState) {
console.warn(`Warning: no outbound prospect state found at ${prospectRoot}; pipeline counts in ${outputPath} will be zero.`);
}📝 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.
| if (regeneratesTrackedBrief && !existsSync(prospectRoot)) { | |
| console.error(`Refusing to regenerate the tracked 11/10 proof-run brief with a zero pipeline: no outbound prospect state found at ${prospectRoot}. Run this command from the service root that holds prospects/, or set SERVICE_REPO_ROOT to it, or pass an explicit --output= under runs/ for a private zero-state report.`); | |
| process.exit(1); | |
| } | |
| if (!existsSync(prospectRoot)) { | |
| console.warn(`Warning: no outbound prospect state found at ${prospectRoot}; pipeline counts in ${outputPath} will be zero.`); | |
| const hasProspectPipelineState = existsSync(prospectRoot) | |
| && listFolders(prospectRoot).some((path) => existsSync(join(path, "pipeline.json"))); | |
| if (regeneratesTrackedBrief && !hasProspectPipelineState) { | |
| console.error(`Refusing to regenerate the tracked 11/10 proof-run brief with a zero pipeline: no outbound prospect state found at ${prospectRoot}. Run this command from the service root that holds prospects/, or set SERVICE_REPO_ROOT to it, or pass an explicit --output= under runs/ for a private zero-state report.`); | |
| process.exit(1); | |
| } | |
| if (!hasProspectPipelineState) { | |
| console.warn(`Warning: no outbound prospect state found at ${prospectRoot}; pipeline counts in ${outputPath} will be zero.`); |
🤖 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/export-market-proof-run.mjs` around lines 41 - 46, Replace the
existsSync(prospectRoot) checks in the tracked-regeneration refusal and warning
branches with a shared predicate that confirms at least one outbound prospect
folder contains pipeline.json. Reuse this predicate for both branches,
preserving the existing refusal and warning behavior. Extend the relevant test
after the private-output case to run the default output and assert it still
refuses.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b57df521d
ℹ️ 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".
| const output = execFileSync("node", args, { | ||
| cwd: serviceRoot, |
There was a problem hiding this comment.
Resolve child scripts from the code checkout
When SERVICE_REPO_ROOT points to a supported data-only service root, setting cwd here makes relative arguments such as scripts/check-market-parity-readiness.mjs resolve inside that data root. Because it has no scripts/ directory, market:proof-run exits with MODULE_NOT_FOUND before generating anything; resolve child entrypoints against the code checkout as runtime-roots.mjs does while retaining serviceRoot as their working/data directory.
Useful? React with 👍 / 👎.
| const regeneratesTrackedBrief = resolve(resolvedOutputPath) === resolve(trackedBriefPath); | ||
| if (regeneratesTrackedBrief && !existsSync(prospectRoot)) { |
There was a problem hiding this comment.
Refuse regeneration when the prospect root is empty
This guard only tests whether prospects/ exists, not whether it contains any outbound pipeline state. A private zero-state run using the suggested --output=runs/... path and the default Loom sheet creates prospects/loom-links.txt; a subsequent default run then bypasses this check and can overwrite the tracked brief with zero counts despite still having no prospect records, recreating the exact silent-clobber scenario this change is intended to prevent.
Useful? React with 👍 / 👎.
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.
|
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.
|
Closing as superseded by
That is visible in the merge conflict itself: every conflicting hunk has an empty The only unique thing left on this branch is an older |
Problem
npm run market:proof-runregenerates the git-tracked 11/10 proof-run brief (growth-brain/ops/11-10-proof-run.md). Running it from any checkout that has no private outbound-prospect state (prospects/, which is gitignored):0with"status": "created"— completely silent,0/5approved,0/40touches,0scored prospects),prospects/prospect-slug|https://www.loom.com/share/...|approved|...placeholder row intoprospects/loom-links.txtthat can be mistaken for an approved proof row.The exporter was also the only tracked-artifact generator not anchored to the service root: its own counters read CWD-relative
prospects/while the sub-processes it shells out to (check-market-parity-readiness,export-growth-metrics) honorSERVICE_REPO_ROOT— so a mixed-root invocation silently reported a wrong zero direction gate.Fix
SERVICE_REPO_ROOTor the invocation directory) vialib/runtime-roots.mjs, matchingexport-internal-dashboard/check-market-parity-readiness.prospects/state — an unavailable pipeline can no longer be mistaken for an empty one. Explicit--output=underruns/still generates zero-state private reports, now with a stderr warning when the prospect root is absent.Tests
scripts/test-direction-proof-gate.mjs: new coverage for the refusal (non-zero exit, tracked brief untouched, noprospects/created) and for CWD-anchoring (running from a different directory still writes into the service root).npm testpasses (137+ checks across all suites, 0 failures).Summary by CodeRabbit
Bug Fixes
Tests