perf(mapper): lazily evaluate Node.js project discovery in MapperContext#156
Conversation
Converts eager and on to lazy, memoized async getters. This avoids executing costly disk I/O for Node.js project discovery on non-Node repositories (like pure Go/Python/Rust), removing startup blockage for other mappers while maintaining shared concurrency for Node-ecosystem mappers.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs maintainer review before merge. Reviewed July 12, 2026, 9:56 AM ET / 13:56 UTC. Summary Reproducibility: yes. Current main directly shows both Node loaders completing before the mapper Promise.all begins, and the PR’s instrumentation demonstrates the resulting unnecessary calls on non-Node repositories. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Merge the current head so non-Node mapping starts immediately while Node-family consumers retain one shared, retryable discovery and task-graph context per mapping run. Do we have a high-confidence way to reproduce the issue? Yes. Current main directly shows both Node loaders completing before the mapper Promise.all begins, and the PR’s instrumentation demonstrates the resulting unnecessary calls on non-Node repositories. Is this the best way to solve the issue? Yes. A per-run memoized context plus a bounded Node-family eligibility gate is narrower and safer than duplicating loaders or changing individual non-Node mappers, and it preserves shared concurrency and retry semantics. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 61ddfe0df86f. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
Keep contributor's lazy-context direction while gating Node-family mappers behind bounded project signals and preserving package-less workspace discovery.\n\nCo-authored-by: Tanmay-008 <tanmayshirbhayye60@gmail.com>
Match the existing bounded project.json discovery scope when deciding whether Node-family mappers are needed.
|
Maintainer repair and exact-head proof for The contributor's lazy-context direction is preserved. The repair prevents all four Node-family mappers from requesting shared Node/Turbo context for pure non-Node repositories, while retaining bounded package-less conventional, nested React, and Nx discovery. Concurrent Node consumers share one project-discovery promise and one task-graph promise; failures are isolated to one mapping run. Proof on the exact head:
Risk: medium because mapper eligibility changes across language families; mitigated by concurrency, failure, retry, multi-root, package-less Node/Nx, Turbo, full-suite, packaged-CLI, and real-repository proof. This head resolves the two prior ClawSweeper findings and supplies the requested runtime/timing proof. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
|
Maintainer repair and exact-head proof for The contributor's lazy-context direction is preserved. The repair prevents all four Node-family mappers from requesting shared Node/Turbo context for pure non-Node repositories, while retaining bounded package-less conventional, nested React, and Nx discovery. Concurrent Node consumers share one project-discovery promise and one task-graph promise; failures are isolated to one mapping run. Proof on the exact head:
Risk: medium because mapper eligibility changes across language families; mitigated by concurrency, failure, retry, multi-root, package-less Node/Nx, Turbo, full-suite, packaged-CLI, and real-repository proof. This head resolves the prior code/proof findings and the release-owned changelog cleanup. Release-note context and @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
2 participants
Summary
Fix eager Node project and Turbo task-graph work for repositories that do not use Node, while preserving the contributor's lazy shared-context design.
The repaired implementation:
Validation
Exact-head proof, instrumentation, real-repository CLI runs, timings, dependency/security audit, and model gate: #156 (comment)
Hosted exact-head gates:
Release-note context and credit
Release-note context is preserved here for the release process and credits
@Tanmay-008. The contributor's original commit remains the first commit in this PR, followed by maintainer repair commits with co-author credit.Fixes #155