fix(skills): remove track-heap-objects from leak hunter workflow - #65
fix(skills): remove track-heap-objects from leak hunter workflow#65Cesar-M-Diaz wants to merge 1 commit into
Conversation
asset-summary does not support heap-profile assets (the type produced by track-heap-objects), so Phase 3 of ns-advanced-memory-leak-hunter failed with 'Unsupported asset type' during baseline-vs-peak hunts. - Leak hunter now captures the peak with heap-sampling only; closure/retainer suspicion is handled by correlating allocator call stacks with runtime-code - Add guardrail documenting the unsupported asset type - Update ns-analyze-asset cross-reference (no longer recommends track-heap-objects) - Drop track-heap-objects mention from ns-memory-spike-analysis bundle description
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughThe memory analysis guidance now requires baseline and peak heap sampling. It removes ChangesMemory Analysis Guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This PR removes an unsupported heap-profile capture path from the leak-hunter workflow and updates related documentation; no actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ns-control-tower
left a comment
ns-control-tower
left a comment
There was a problem hiding this comment.
Walkthrough
This docs-only PR removes track-heap-objects from the ns-advanced-memory-leak-hunter workflow because the resulting heap-profile asset is not supported by asset-summary (the tool consumed during baseline-vs-peak hunts), causing Phase 3 to fail with "Unsupported asset type". The peak is now captured with heap-sampling only, closure/retainer suspicion is handled by correlating allocator call stacks with runtime-code, a new guardrail documents the unsupported asset type, and the two bundle.json descriptions plus the ns-analyze-asset cross-reference are updated to stay consistent.
Changes
| File(s) | Summary |
|---|---|
| bundle.json, packages/core/bundle.json | Drop the track-heap-objects mention from the ns-memory-spike-analysis description; both copies kept in sync. |
| skills/ns-advanced-memory-leak-hunter/SKILL.md | Phase 3 reworked to use heap-sampling only; added a guardrail explaining asset-summary does not support heap-profile assets. |
| skills/ns-analyze-asset/SKILL.md | Follow-up recommendation no longer points to track-heap-objects; now recommends a deeper baseline-vs-peak heap-sampling hunt. |
Assessment
- No code, security, async, or performance surfaces — this is a markdown/JSON skill-description change only, so no sandbox validation was warranted.
- 🧹 One non-blocking nitpick on
ns-analyze-asset/SKILL.md: after the edit, the two consecutive follow-up bullets both point tons-advanced-memory-leak-hunter, making the second bullet redundant. Optional tidy. - The two
bundle.jsonfiles were correctly kept in sync (identical edits), avoiding a drift between the root andpackages/corecopies.
Verdict: APPROVE — clean docs-only fix; the only finding is a non-blocking readability nitpick.
| - If allocation stack traces are insufficient, recommend a deeper leak hunt (baseline vs peak heap sampling) via `ns-advanced-memory-leak-hunter`. | ||
| - For deeper leak hunting workflows, reference the `ns-advanced-memory-leak-hunter` skill. |
There was a problem hiding this comment.
🧹 Nitpick
After this edit both bullets now point to ns-advanced-memory-leak-hunter, making the second bullet largely redundant with the first. Consider merging them into one recommendation, or dropping the second bullet entirely.
| - If allocation stack traces are insufficient, recommend a deeper leak hunt (baseline vs peak heap sampling) via `ns-advanced-memory-leak-hunter`. | |
| - For deeper leak hunting workflows, reference the `ns-advanced-memory-leak-hunter` skill. | |
| - If allocation stack traces are insufficient, recommend a deeper leak hunt (baseline vs peak heap sampling) via `ns-advanced-memory-leak-hunter`. |
(non-blocking — purely a readability tidy)
2 participants
asset-summary does not support heap-profile assets (the type produced by track-heap-objects), so Phase 3 of ns-advanced-memory-leak-hunter failed with 'Unsupported asset type' during baseline-vs-peak hunts.
Summary by CodeRabbit