Skip to content

refactor(onboard): inline compatibility result sequences - #5646

Merged
cv merged 12 commits into
mainfrom
refactor/onboard-fsm-compat-result-loop
Jun 24, 2026
Merged

refactor(onboard): inline compatibility result sequences#5646
cv merged 12 commits into
mainfrom
refactor/onboard-fsm-compat-result-loop

Conversation

@cv

@cv cv commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remove the bulk compatibility-result recorder from the onboarding runtime boundary. Callers now iterate explicit result sequences themselves, keeping the compatibility bridge surface smaller while preserving existing replay behavior.

Changes

  • Delete recordStateResultsWithStepCompatibility from OnboardRuntimeBoundary.
  • Inline the result loops in the runtime-boundary and resume-repair tests that used the helper.
  • Keep the singular compatibility recorder as the only compatibility result entrypoint.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Refactor
    • Simplified internal recording logic for state transitions by removing batch processing in favor of sequential individual recording, improving code clarity and maintainability.

cv added 6 commits June 22, 2026 22:46
@cv cv self-assigned this Jun 23, 2026
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review 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: Enterprise

Run ID: f29e4e99-2554-4d28-9ed0-45e7cd9c792f

📥 Commits

Reviewing files that changed from the base of the PR and between a309c1b and 377c6ee.

📒 Files selected for processing (3)
  • src/lib/onboard/resume-machine-repair.test.ts
  • src/lib/onboard/runtime-boundary.test.ts
  • src/lib/onboard/runtime-boundary.ts
💤 Files with no reviewable changes (1)
  • src/lib/onboard/runtime-boundary.ts

📝 Walkthrough

Walkthrough

The batch method recordStateResultsWithStepCompatibility is removed from OnboardRuntimeBoundary. Two test call sites are updated: resume-machine-repair.test.ts builds a results array and loops over it, and runtime-boundary.test.ts calls recordStateResultWithStepCompatibility twice sequentially.

Changes

Remove batch compatibility helper and update call sites

Layer / File(s) Summary
Remove batch method and update call sites
src/lib/onboard/runtime-boundary.ts, src/lib/onboard/resume-machine-repair.test.ts, src/lib/onboard/runtime-boundary.test.ts
recordStateResultsWithStepCompatibility is deleted from OnboardRuntimeBoundary; the repair test now builds a results array and records each entry individually, and the boundary test replaces the batched call with two sequential single-result calls.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

v0.0.67

Poem

A batch of results once marched in a row,
Now each one steps forward with its own little flow.
The loop is unwound, the array set free,
One call at a time — just as it should be!
🐇✨ Sequential steps, hopping along merrily!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: inlining compatibility result sequences by removing the batch helper method and having callers iterate individually.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/onboard-fsm-compat-result-loop

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

@github-code-quality

github-code-quality Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the refactor/onboard-fsm... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/onboard-fsm... 377c6ee +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the refactor/onboard-fsm... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/onboard-fsm... 377c6ee +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 18%

Updated June 24, 2026 03:16 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: onboard-resume-e2e, onboard-repair-e2e, cloud-onboard-e2e
Optional E2E: onboard-negative-paths-e2e

Dispatch hint: onboard-resume-e2e,onboard-repair-e2e,cloud-onboard-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e (medium): Required by the onboarding resume compatibility rule because this PR changes onboarding runtime boundary state-result compatibility used during resume/replay paths.
  • onboard-repair-e2e (medium): Required by the onboarding resume compatibility rule because resume repair/replay behavior is adjacent to the changed runtime-boundary compatibility bridge and tests touch resume-machine repair coverage.
  • cloud-onboard-e2e (high): Required because the changed runtime boundary participates in full hosted onboarding state transitions and completion; this validates the real cloud onboarding flow against the live CLI/sandbox path.

Optional E2E

  • onboard-negative-paths-e2e (medium): Optional adjacent confidence for onboarding failure/skip behavior, since runtime-boundary compatibility logic handles stale or invalid transition results; not required because the PR is specifically covered by resume, repair, and cloud onboarding E2Es.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: onboard-resume-e2e,onboard-repair-e2e,cloud-onboard-e2e

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: onboard-resume-vitest, onboard-repair-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • onboard-resume-vitest: Changes touch onboarding runtime-boundary state-result compatibility and resume-machine repair coverage, which can affect live onboarding state transitions and persisted-session resume behavior. The onboarding resume compatibility rule requires the onboard resume Vitest job.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • onboard-repair-vitest: The changed runtime-boundary compatibility path and resume-machine repair tests can affect repair/backstop execution from persisted sessions, so repair is required rather than optional for this resume state-machine path.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • src/lib/onboard/resume-machine-repair.test.ts
  • src/lib/onboard/runtime-boundary.test.ts
  • src/lib/onboard/runtime-boundary.ts

@cv cv added the v0.0.67 label Jun 23, 2026
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change labels Jun 23, 2026
@jyaunches jyaunches added v0.0.68 and removed v0.0.67 labels Jun 24, 2026
Base automatically changed from refactor/onboard-fsm-live-slice-inputs to main June 24, 2026 03:10
@cv
cv merged commit 169b884 into main Jun 24, 2026
44 checks passed
@cv
cv deleted the refactor/onboard-fsm-compat-result-loop branch June 24, 2026 03:18
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the bulk compatibility-result recorder from the onboarding
runtime boundary. Callers now iterate explicit result sequences
themselves, keeping the compatibility bridge surface smaller while
preserving existing replay behavior.

## Changes
- Delete `recordStateResultsWithStepCompatibility` from
`OnboardRuntimeBoundary`.
- Inline the result loops in the runtime-boundary and resume-repair
tests that used the helper.
- Keep the singular compatibility recorder as the only compatibility
result entrypoint.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Simplified internal recording logic for state transitions by removing
batch processing in favor of sequential individual recording, improving
code clarity and maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Hadar Cohen <hacohen@redhat.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the bulk compatibility-result recorder from the onboarding
runtime boundary. Callers now iterate explicit result sequences
themselves, keeping the compatibility bridge surface smaller while
preserving existing replay behavior.

## Changes
- Delete `recordStateResultsWithStepCompatibility` from
`OnboardRuntimeBoundary`.
- Inline the result loops in the runtime-boundary and resume-repair
tests that used the helper.
- Keep the singular compatibility recorder as the only compatibility
result entrypoint.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Simplified internal recording logic for state transitions by removing
batch processing in favor of sequential individual recording, improving
code clarity and maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Hadar Cohen <hacohen@redhat.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the bulk compatibility-result recorder from the onboarding
runtime boundary. Callers now iterate explicit result sequences
themselves, keeping the compatibility bridge surface smaller while
preserving existing replay behavior.

## Changes
- Delete `recordStateResultsWithStepCompatibility` from
`OnboardRuntimeBoundary`.
- Inline the result loops in the runtime-boundary and resume-repair
tests that used the helper.
- Keep the singular compatibility recorder as the only compatibility
result entrypoint.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Simplified internal recording logic for state transitions by removing
batch processing in favor of sequential individual recording, improving
code clarity and maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants