Skip to content

fix(checks): recover inconclusive probe handling - #203

Draft
christian-byrne wants to merge 6 commits into
mainfrom
christian-byrne/cmprec-22-harness
Draft

christian-byrne wants to merge 6 commits into
mainfrom
christian-byrne/cmprec-22-harness

Conversation

@christian-byrne

@christian-byrne christian-byrne commented Sep 17, 2026

Copy link
Copy Markdown
Contributor
  • Recover standalone check-script fixes.
  • Missing evidence stays inconclusive.
  • Draft; fresh review still required.

Human owner: christian-byrne

Full context for agent readers

Recovery scope

Recovers import-graph and stateless-probe fixes from the preserved frontend migration parent, directly onto standalone comfy-multi-player main. Frontend relocation remains deferred. The private parent map identifies these source portions as H036, H040 and H066 (source hunk identifiers).

At published head, malformed dependency-cruiser summaries, negative work counts, malformed violation rows, missing Vitest/probe, spawn failure and signal termination exit 2 (inconclusive). Genuine findings and failing probes remain failures. Tests execute actual scripts in isolated fixtures. The forward merge preserves standalone main's fixture-root governance and this replacement's missing-probe protections together. No runtime dependency was added.

Verification

Normally integrated standalone governance main, preserving both histories without conflicts or corrective source changes. Node 22.22.2; existing identical-lockfile dependencies reused without installation. The integrated head passes 1,037 tests across 80 files, 29 focused tests, complete types/build and all structural package gates, including all six remotely verified source pins. Coordinator independently reran the 29 focused tests and complete typechecks. Lint has zero errors and existing warnings. New-head hosted checks and substantive fresh review remain separate. No new mutation-score or consumer-testing claim.

Review continuity

The additional three-record carryover packet also remains preserved. Its standalone npm remediation was rechecked after integration; frontend-specific obligations remain separate.

Original authors, replies, source anchors, branches and review records stay intact; no historical approval transfers. Other portable-harness findings belong to the separate replacement, not this script slice. No operation semantics, exports, dependencies, widget catalog or release version changed. No package publication, deployment or source retirement.

Glossary: CMP = comfy-multi-player; inconclusive = unavailable or malformed evidence, never a passing check.

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation for import-graph reports, including malformed or incomplete data.
    • Checks now distinguish inconclusive results from genuine failures when required tools, probes, or report data are unavailable.
    • Improved handling of execution errors and signal termination during stateless checks.
    • Prevented misleading runtime errors by reporting invalid prerequisites and inputs clearly.
  • Tests

    • Added comprehensive coverage for malformed reports, missing dependencies, execution failures, and successful validation scenarios.

@christian-byrne christian-byrne self-assigned this Sep 17, 2026
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 54c3cf62-0791-4ae8-a53c-b5c1aa14e89d

📥 Commits

Reviewing files that changed from the base of the PR and between 3bddfbd and a1be1b0.

📒 Files selected for processing (4)
  • scripts/check-import-graph.mjs
  • scripts/check-stateless.mjs
  • test/check-import-graph.test.ts
  • test/check-stateless.test.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

The gates now validate dependency-cruiser reports and stateless-check prerequisites before evaluation. Invalid inputs, missing files, spawn errors, and signal termination return exit code 2. Tests cover inconclusive results and preserve exit code 1 for genuine failures.

Changes

Gate validation

Layer / File(s) Summary
Dependency report validation
scripts/check-import-graph.mjs, test/check-import-graph.test.ts
The import-graph gate validates report structure and values before reading summary data. Parameterized tests cover malformed reports and require exit code 2 without a TypeError.
Stateless probe validation
scripts/check-stateless.mjs, test/check-stateless.test.ts
The stateless gate checks required executables and files, handles spawn errors and signal termination as inconclusive, and uses isolated fixtures to test success and failure outcomes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a1be1

The gate changes preserve distinct outcomes for invalid evidence, failed checks, and successful checks, with focused coverage described for each path. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: recovering inconclusive handling in the check scripts. It is concise and specific to the pull request.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the risk:R1 PR risk grade (advisory shadow check; grader-owned) label Sep 17, 2026
@christian-byrne

Copy link
Copy Markdown
Contributor Author

Harness recovery: source findings preserved with attribution. No historical approval transfers.

Full context for agent readers

This packet accounts for 10 source inline records for standalone harness recovery. Frontend relocation remains deferred. Original threads and every original routing destination remain intact.

scripts/check-import-graph.mjs

CodeRabbit finding, 2026-09-03T15:50:18Z: fixed-and-reverified at replacement commit. Required summary fields and malformed rows are validated before access; coordinator's 23 focused tests pass, including malformed evidence exit 2.

scripts/check-stateless.mjs

CodeRabbit finding, 2026-09-03T15:50:18Z: fixed-and-reverified at replacement commit. Missing executable/file, spawn errors and signal termination exit 2; real lint/probe findings exit 1 and clean evidence exits 0. Coordinator executed these focused cases.

test/check-import-graph.test.ts

CodeRabbit finding, 2026-09-03T15:50:19Z: fixed-and-reverified at replacement commit. NodeJS.ProcessEnv now models optional environment values; complete typecheck passes. The earlier annotation also compiled, so this is a type-model correction, not a reproduced compiler failure. No human decision needed.

.agents/checks/import-graph.md

CodeRabbit finding, 2026-09-04T18:54:46Z: superseded-with-reason at replacement commit. The frontend pnpm instruction applies to its workspace, not standalone CMP. CMP tracks package-lock.json and its existing import-graph profile uses npm ci/npm run check:imports. Preserve npm here and preserve the original frontend finding; no package-manager choice is reopened.

Coordinator read all 34 selected complete bodies and verified identities, authors, dates, body equality, anchors and original destinations. Bounded refresh found no edits or new replies. Three further threads (nine records) remain explicitly unadjudicated; this is not whole-parent closure. Complete private source bodies and immutable ledger remain unchanged.

For the check-script replacement, 23 focused tests and complete typechecks passed on the published head; the preceding runtime-identical head passed 1,027 full tests. For the portable harness replacement, no new tests ran in this pass. Fresh hosted review and consumer verification remain separate.

Glossary: CMP = comfy-multi-player; disposition = treatment at this replacement only; inconclusive = missing or malformed evidence, never a passing check.

@christian-byrne

Copy link
Copy Markdown
Contributor Author
  • Three original remediation records preserved.
  • Standalone npm guidance remains correct.
  • Historical deferral grants no migration authority.
Full context for agent readers
  1. CodeRabbit, September 3: use pnpm in frontend remediation. Superseded for this standalone destination only. At the inspected replacement head, scripts/check-stateless.mjs:30-35,69 correctly tells contributors to use npm ci: this repository has package-lock.json. The frontend pnpm requirement remains preserved as source-specific context.
  2. Agent posting under Christian's account, September 3: defer the fix until import lands. Context only. The old migration-dependent deferral is not permission to resume migration or postpone a valid standalone fix.
  3. CodeRabbit, September 3: acknowledgment and stored import guidance. Context only. The stored learning explicitly concerns zero-diff frontend imports, which are now deferred. It is neither current authority nor approval of this replacement.

The bounded refresh found no new replies or body edits in this source thread. Original author/date/body, reply relationships, original/current commit anchors and all other destination routes remain preserved. This pass inspected committed source, not a new test run. After integrating the subsequently merged standalone governance change, reconfirm stateless precondition/probe tests, complete types, substantive exact-head review and hosted checks. No source thread is being resolved and no historical approval or QA result transfers.

CMP means standalone comfy-multi-player. QA means quality assurance and test evidence.

@christian-byrne

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Review the standalone harness recovery at the current head. The source-review carryover remains below; frontend relocation stays deferred.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

@christian-byrne: I will review the standalone harness recovery at the current head. I will treat the frontend relocation as out of scope.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:R1 PR risk grade (advisory shadow check; grader-owned)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant