fix(logs): preserve sparse-source breadcrumbs in nemoclaw <sandbox> logs - #5711
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
📝 WalkthroughWalkthrough
Quota-based tail merge for gateway log surfacing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/domain/sandbox/logs.ts (1)
180-212: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueQuota algorithm correctly enforces the
≤ maxLinescap.The bound holds:
reserved.size ≤ nonEmptyCount * floor(maxLines / nonEmptyCount) ≤ maxLines, soremainingis never negative and the final set never exceedsmaxLines. Identity-basedSet<ScoredLine>and the chronological re-sort are sound.One optional cleanup:
perSource.flat()is recomputed on both themaxLines <= 0path (Line 181) and the remaining-fill path (Line 199). Minor, but you could hoist it once if you want to avoid the duplicate traversal.🤖 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 `@src/lib/domain/sandbox/logs.ts` around lines 180 - 212, The perSource.flat() method is being called twice in this function, once on line 181 in the maxLines <= 0 branch and again on line 199 when creating the candidates array, causing unnecessary traversal of the same data. Extract perSource.flat() into a single variable declared before the maxLines <= 0 conditional check, and then reuse that variable in both the early return path and when filtering candidates for the remaining quota allocation.
🤖 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.
Nitpick comments:
In `@src/lib/domain/sandbox/logs.ts`:
- Around line 180-212: The perSource.flat() method is being called twice in this
function, once on line 181 in the maxLines <= 0 branch and again on line 199
when creating the candidates array, causing unnecessary traversal of the same
data. Extract perSource.flat() into a single variable declared before the
maxLines <= 0 conditional check, and then reuse that variable in both the early
return path and when filtering candidates for the remaining quota allocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4e3b06e7-b4e9-4efa-8435-ad84143ce46d
📒 Files selected for processing (3)
src/lib/actions/sandbox/logs.test.tssrc/lib/domain/sandbox/logs.test.tssrc/lib/domain/sandbox/logs.ts
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
cjagwani
left a comment
There was a problem hiding this comment.
Took a look at this. The fix actually solves the breadcrumb-drop from #5685, sparse sources like the Telegram bridge watchdog don't get squeezed out by a chatty OpenShell source anymore. Single-source case behaves the same as before. LGTM.
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - #5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - #5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - #5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - #5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - #5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - #5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - #5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - #5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - #5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - #5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - #5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - #5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - #5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - #5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - #5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - #5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - #5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release-notes prose; no runtime behavior changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## 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 - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] 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) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ogs (NVIDIA#5711) ## Summary `nemoclaw <sandbox> logs` previously dropped sparse-source breadcrumbs (e.g. the Telegram bridge watchdog message `bridge did not start within 15s`) when the OpenShell source emitted enough recent activity to dominate the global tail-clip. Each non-empty source now gets a floor of `floor(maxLines / non-empty-source-count)` of its most recent lines, so a quiet gateway log is no longer silently squeezed out by a chatty audit stream. Strict `≤ maxLines` is preserved. ## Related Issue Fixes NVIDIA#5685 ## Changes - `src/lib/domain/sandbox/logs.ts` — `mergeTailLogLines` rewritten with a per-source floor; remaining slots up to `maxLines` are filled by the most recent lines across all sources, then the selection is sorted chronologically. Extracted `sortChronologically` helper. - `src/lib/domain/sandbox/logs.test.ts` — added unit cases for sparse-source preservation, untimestamped diagnostic inheritance from a preceding gateway timestamp, and the strict `≤ maxLines` guarantee under two chatty sources. - `src/lib/actions/sandbox/logs.test.ts` — added a behavioural case that mocks the OpenClaw gateway tail + OpenShell logs probe with a sparse-old gateway and dense-recent OpenShell, asserting both the watchdog breadcrumb and the boot line reach the merged stdout. ## 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: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added gateway breadcrumb display in sandbox logs when output is dominated by other sources. * **Bug Fixes** * Improved log merging to preserve sparse content from less-active sources. * Fixed timestamp inheritance for diagnostic lines across sources. * **Improvements** * Enhanced log output balancing when merging multiple sources. * Optimized maximum line limits to prevent dominant sources from crowding sparse ones. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
## Summary Adds the v0.0.69 release notes to the published release-notes page so users can see the shipped sandbox recovery, Deep Agents Code, Hermes, inference, policy, and release-validation changes. The section is based on the v0.0.69 announcement and links each user-facing theme to the deeper docs pages that already cover the behavior. ## Changes - Added a new `v0.0.69` section to `docs/about/release-notes.mdx`. - Linked release-note themes to lifecycle, backup, troubleshooting, Deep Agents Code, commands, workspace, messaging, Hermes, inference, security, monitoring, and network-policy docs. Source summary: - NVIDIA#5455 -> `docs/about/release-notes.mdx`: Summarized persistent workspace and state cleanup during sandbox destroy. - NVIDIA#5738 -> `docs/about/release-notes.mdx`: Summarized nonzero exit status preservation for failed hosted endpoint validation. - NVIDIA#5786 -> `docs/about/release-notes.mdx`: Summarized live sandbox rediscovery when local registry state is missing. - NVIDIA#5881 -> `docs/about/release-notes.mdx`: Summarized the `nemo-deepagents` alias command surface. - NVIDIA#5594 -> `docs/about/release-notes.mdx`: Summarized the Hermes Agent 2026.6.19 update. - NVIDIA#5777 -> `docs/about/release-notes.mdx`: Summarized manifest-derived messaging channel support. - NVIDIA#5825 -> `docs/about/release-notes.mdx`: Summarized DeepSeek V4 Flash managed-vLLM defaults for DGX Station. - NVIDIA#5877 -> `docs/about/release-notes.mdx`: Summarized provider switch metadata preservation. - NVIDIA#5932 -> `docs/about/release-notes.mdx`: Summarized transient inference smoke retry behavior. - NVIDIA#5934 -> `docs/about/release-notes.mdx`: Summarized constrained inference smoke retry boundaries. - NVIDIA#5681 -> `docs/about/release-notes.mdx`: Summarized Shields config-hash sealing during auto-restore. - NVIDIA#5682 -> `docs/about/release-notes.mdx`: Summarized sandbox connect process-limit enforcement. - NVIDIA#5683 -> `docs/about/release-notes.mdx`: Summarized JSON agent failure provenance warnings. - NVIDIA#5711 -> `docs/about/release-notes.mdx`: Summarized sparse-source log breadcrumbs. - NVIDIA#5838 -> `docs/about/release-notes.mdx`: Summarized host-authoritative Shields status. - NVIDIA#5880 -> `docs/about/release-notes.mdx`: Summarized policy round-trip documentation updates. - NVIDIA#5886 -> `docs/about/release-notes.mdx`: Summarized network request approval-flow documentation updates. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release-notes prose; no runtime behavior changed. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## 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 - [ ] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] 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) `npm run docs` passed with 0 errors and the existing Fern light-mode accent contrast warning. `fern check --warnings` reported the same accent-color warning. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for **v0.0.69**, covering improved sandbox lifecycle recovery (state preservation across destroy/recreate/rebuild/recovery/validation failures), clearer Deep Agents Code terminal/CLI behavior, and safer Hermes messaging/provider switching with manifest-driven channels. * Improved inference setup validation guidance, including handling of local/compatible endpoints and redaction of sensitive validation errors. * Refreshed release-gate documentation with clearer approval examples and validation behavior for NVIDIA API keys vs hosted inference keys. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
nemoclaw <sandbox> logspreviously dropped sparse-source breadcrumbs (e.g. the Telegram bridge watchdog messagebridge did not start within 15s) when the OpenShell source emitted enough recent activity to dominate the global tail-clip. Each non-empty source now gets a floor offloor(maxLines / non-empty-source-count)of its most recent lines, so a quiet gateway log is no longer silently squeezed out by a chatty audit stream. Strict≤ maxLinesis preserved.Related Issue
Fixes #5685
Changes
src/lib/domain/sandbox/logs.ts—mergeTailLogLinesrewritten with a per-source floor; remaining slots up tomaxLinesare filled by the most recent lines across all sources, then the selection is sorted chronologically. ExtractedsortChronologicallyhelper.src/lib/domain/sandbox/logs.test.ts— added unit cases for sparse-source preservation, untimestamped diagnostic inheritance from a preceding gateway timestamp, and the strict≤ maxLinesguarantee under two chatty sources.src/lib/actions/sandbox/logs.test.ts— added a behavioural case that mocks the OpenClaw gateway tail + OpenShell logs probe with a sparse-old gateway and dense-recent OpenShell, asserting both the watchdog breadcrumb and the boot line reach the merged stdout.Type of Change
Verification
Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Improvements