Skip to content

fix(perf): stop loading hard-link-directory on every bit invocation; self-diagnosing fs-read guard - #10599

Draft
luvkapur wants to merge 3 commits into
masterfrom
fix/fs-read-guard-status-diagnostics
Draft

fix(perf): stop loading hard-link-directory on every bit invocation; self-diagnosing fs-read guard#10599
luvkapur wants to merge 3 commits into
masterfrom
fix/fs-read-guard-status-diagnostics

Conversation

@luvkapur

@luvkapur luvkapur commented Aug 11, 2026

Copy link
Copy Markdown
Member

Why

e2e_test_bbit (nightly, 32 nodes) has been red since 2026-07-23 — 19 of the last 21 failures are one single test out of ~2,900: the bit status filesystem-read guard (MAX_FILES_READ_STATUS = 1515, actual 1532 → 1541 and creeping).

Root cause

Diffing BIT_DEBUG_READ_FILE output of bvm-installed 2.0.33 (last green) vs 2.0.35 (first red) — only two commits apart — shows no new runtime reads. Instead, #10515's dependency change reshaped the released bundle's pnpm hoisted layout: @teambit/toolbox.fs.hard-link-directory plus a full private fs-extra tree now load from a nested copy under the compiler package instead of a shared hoisted copy (~+21 reads). The nightly +1–3 creep is the same mechanism — every nightly bundle re-resolves latest transitive deps, and hoisting shifts occasionally add nested duplicates (p-limit, pify, ssri, mimic-fn appeared between 2.0.35 and 2.0.74).

Changes

  1. compiler.task.ts: lazy-import hard-link-directory — it is used only inside the build task, yet its top-level import pulled ~36 files into every bootstrap/bit status in the bundle layout. Verified the compiled dist defers the require until the task executes. This alone puts the bundle back under the original threshold.
  2. The bit status guard now counts unique logical modules, not raw physical reads — paths are normalized past the last node_modules/ segment, so duplicate copies of the same module (the entire layout-drift noise class) count once. Measured on the incident itself: 2.0.33 → 2.0.35 physical files went 1836 → 1857 while logical modules stayed exactly 1625 → 1625 — the three-week red was 100% duplication, 0% new code. The metric converges for both install flavors, so one snapshot (files-snapshot-status.txt, logical ids from the released linux-x64 bundle in a clean container) serves repo and bundle runs alike. No drift headroom needed: threshold 1700 vs measured baselines 1625–1645, to be tightened to measured+~30 from the report line below.
  3. Failures are self-diagnosing, greens are self-measuring — a red prints which modules are new vs the snapshot plus the full current list (so any red carries the data to regenerate an exact snapshot); every run prints a total reads / physical files / logical modules / duplicate copies report line, accumulating true per-environment baselines in CI logs and keeping duplication trends visible without failing anyone. Verified against captured outputs of 2.0.33/2.0.35/2.0.74 (mac + clean linux container): all pass, an injected synthetic regression is pinpointed by name.

Verification

  • npm run lint green (oxlint + tsc).
  • Snapshot-diff logic exercised against real captured outputs of 2.0.33 / 2.0.35 / 2.0.74 (see numbers above).
  • bit compile teambit.compilation/compiler — dist verified lazy: await Promise.resolve().then(() => require('@teambit/toolbox.fs.hard-link-directory')).

🤖 Generated with Claude Code

…make fs-read guard self-diagnosing

The nightly e2e_test_bbit job has been red since 2026-07-23: the released
bundle's 'bit status' crossed the MAX_FILES_READ_STATUS=1515 guard (1532,
creeping to 1541 by 2.0.74). Root cause, found by diffing BIT_DEBUG_READ_FILE
output of bvm-installed 2.0.33 (last green) vs 2.0.35 (first red): #10515's
dependency change reshaped the bundle's pnpm hoisted layout, so
@teambit/toolbox.fs.hard-link-directory plus a private fs-extra tree load from
a nested copy under the compiler package instead of a shared hoisted copy
(~+21 reads). The nightly creep (+1-3) is the same mechanism - each nightly
re-resolves transitive deps and hoisting shifts add nested duplicates.

Three changes:
- compiler.task.ts imports hard-link-directory lazily inside the build task -
  it is build-time-only, yet its top-level import pulled ~36 files into every
  bootstrap/status in the bundle layout.
- the bit-status guard failure now prints WHICH files are new vs a snapshot
  (files-snapshot-status.txt, generated from the released 2.0.74 bundle),
  same as the bootstrap guard - a red is a 2-minute diagnosis, not a 3-week
  mystery. Layout-mismatched diffs (repo vs bundle flavor) degrade to a
  count + sample instead of noise.
- threshold 1515 -> 1650: headroom for bundle-layout drift, which the diff in
  the error message now makes visible and attributable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Lazy-load hard-link-directory and add self-diagnosing fs-read guard for bit status

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Lazy-import hard-link-directory in CompilerTask to reduce baseline startup filesystem reads.
• Make bit status fs-read guard self-diagnosing by diffing against a snapshot file.
• Add bundle-based bit status snapshot and raise the guard threshold to 1650 for drift headroom.
Diagram

graph TD
  CLI["Bit CLI invocation"] --> Envs["Envs load"] --> CompilerTask["CompilerTask module"]
  CompilerTask --> Build["Build task executes"] --> LazyImport["Lazy import hard-link-directory"]
  CLI --> Status["bit status"] --> DebugOut["BIT_DEBUG_READ_FILE output"] --> E2E["FS-read e2e guard"] --> Snapshot[("Snapshot files")] --> Error["Diff-based failure message"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Stabilize bundle hoisting/layout (pin transitive deps / pnpm settings)
  • ➕ Reduces read-count drift without raising thresholds
  • ➕ Avoids nested duplicate dependency trees in the released bundle
  • ➖ Hard to guarantee across environments and release pipelines
  • ➖ Can constrain dependency updates and increase maintenance overhead
2. Split build-only artifact linking into a separate module/package
  • ➕ Guarantees build-only deps never affect bit status/bootstrap load
  • ➕ Creates a clean separation between runtime and build-time concerns
  • ➖ Larger refactor (APIs, packaging, and build graph changes) than needed for the incident fix
3. Change the guard metric (module-load tracing vs filesystem reads)
  • ➕ Less sensitive to node_modules layout/hoisting differences
  • ➕ More directly tied to code-path execution than file layout
  • ➖ More complex instrumentation and potential for false positives/negatives
  • ➖ Moves away from measuring the actual I/O cost the guard is intended to cap

Recommendation: The PR’s approach is the best near-term fix: lazy-loading removes an avoidable always-on dependency cost, and the guard becomes actionable by reporting a snapshot diff. Keeping the file-read metric preserves the original intent (cap I/O) while the added diagnostics and increased headroom make layout-driven drift debuggable rather than mysterious. Longer-term, consider stabilizing bundle hoisting/layout if read-count drift continues to trend upward.

Files changed (3) +1914 / -10

Bug fix (1) +5 / -1
compiler.task.tsLazy-import hard-link-directory to avoid startup file reads +5/-1

Lazy-import hard-link-directory to avoid startup file reads

• Removes the top-level import of '@teambit/toolbox.fs.hard-link-directory' and performs a dynamic import inside the build-only hard-linking method. This prevents pulling the package (and its nested 'fs-extra' tree in bundle layouts) into every Bit invocation.

scopes/compilation/compiler/compiler.task.ts

Tests (2) +1909 / -9
files-snapshot-status.txtAdd released-bundle snapshot for 'bit status' filesystem-read baseline +1865/-0

Add released-bundle snapshot for 'bit status' filesystem-read baseline

• Introduces a new snapshot file capturing the expected file-read set for 'bit status' in the released (hoisted) bundle layout. Used by the e2e guard to compute and print newly loaded files when regressions occur.

e2e/performance/files-snapshot-status.txt

filesystem-read.e2e.tsMake 'bit status' fs-read guard self-diagnosing and raise threshold +44/-9

Make 'bit status' fs-read guard self-diagnosing and raise threshold

• Raises 'MAX_FILES_READ_STATUS' to 1650 and documents the hoisting/layout-driven root cause of recent failures. Enhances guard errors to diff against a snapshot (bootstrap vs status) and to degrade to a count + sample when snapshot/run layouts are incompatible.

e2e/performance/filesystem-read.e2e.ts

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Status snapshot seems mismatched ✓ Resolved 🐞 Bug ☼ Reliability
Description
files-snapshot-status.txt contains 1,865 entries, while the test comments describe the released
bundle as ~1,540 reads and the guard threshold is 1,650; this strongly suggests the snapshot was
generated from a different flavor/run than what the test documents. If/when the bit status guard
fails, getNewlyLoadedFiles() diagnostics can become misleading or miss the actual newly-loaded
files you expect it to highlight for that environment.
Code

e2e/performance/files-snapshot-status.txt[R1862-1865]

+lodash.compact/index.js
+@teambit/graph/dist/component-id-graph.js
+@teambit/status/dist/status-formatter.js
+@teambit/cli/dist/pager.js
Evidence
The test explicitly documents an expected released-bundle read count (~1,540) and sets a threshold
of 1,650, but the newly-added status snapshot file itself has 1,865 entries, indicating the snapshot
likely doesn’t represent the documented baseline/flavor and can therefore mislead diagnostics.

e2e/performance/filesystem-read.e2e.ts[14-35]
e2e/performance/filesystem-read.e2e.ts[85-99]
e2e/performance/files-snapshot-status.txt[1851-1865]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`files-snapshot-status.txt` appears inconsistent with the documented expected `bit status` read counts and the configured threshold, which can reduce the usefulness of the self-diagnosing diff when the guard trips.
### Issue Context
- The test’s `MAX_FILES_READ_STATUS` is set to 1650 and comments state the released/hoisted bundle is ~1540 reads.
- The committed `files-snapshot-status.txt` currently contains 1865 lines (entries).
- `getNewlyLoadedFiles(output, 'files-snapshot-status.txt')` uses this file only for diagnostics, so an overinclusive/mismatched snapshot makes failure output noisy or misleading.
### Fix Focus Areas
- e2e/performance/filesystem-read.e2e.ts[14-99]
- e2e/performance/files-snapshot-status.txt[1-1865]
### Suggested fix
1. Regenerate `files-snapshot-status.txt` using the same environment/flavor the comments describe (released bundle hoisted layout vs repo `.pnpm` layout), and ensure it corresponds to the expected baseline.
2. Align the inline documentation (expected counts) with the regenerated snapshot.
3. Optionally add a lightweight sanity check (comment or assertion) to prevent committing a status snapshot whose size is wildly inconsistent with the expected baseline/threshold.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread e2e/performance/files-snapshot-status.txt
@luvkapur
luvkapur marked this pull request as draft August 11, 2026 19:53
luvkapur and others added 2 commits August 11, 2026 16:00
…ten threshold to 1600; print full file list on failure

Addresses the Qodo finding on #10599: the snapshot was generated from a local
macOS run of the released bundle - correct flavor, but a superset of what CI's
leaner fixture reads. Regenerated inside a linux/amd64 node:22.22.0 container
from the released linux-x64 bundle (1,878 entries; the mac and linux sets
share 1,860 of ~1,870 files, so flavor was never the issue - offline supersets
are inherent). Two hardenings follow from accepting that limit:

- the guard now calls makeSnapshot() on failure, printing the complete current
  install-file list, so every red carries the exact data to inspect masked
  files and regenerate a perfect snapshot from real CI output
- threshold 1650 -> 1600: the post-fix baseline is a projection (1541 - ~36
  lazy-import reads = ~1505) and drift events arrive in ~+20 lumps; 1600 =
  projection + error margin + two events. Re-tighten to measured + ~50 once
  post-fix nightlies establish the real baseline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l reads

Replaces the threshold-bump approach. The raw-reads metric conflated two
things: bit loading more code (the regression the guard exists to catch) and
pnpm hoisting layout drift duplicating physical copies of the same modules
(packaging noise that reshuffles every nightly bundle). Measured across the
incident that motivated this: 2.0.33 -> 2.0.35 physical unique files went
1836 -> 1857 (dup copies 211 -> 232) while logical modules stayed exactly
1625 -> 1625 - the three-week nightly red was 100% duplication noise, 0% new
code.

The guard now counts unique logical modules (paths normalized past the last
node_modules/ segment), which:
- is immune to the entire layout-drift noise class - no headroom needed, the
  interim 1650/1600 bumps are gone
- converges for both install flavors (repo .pnpm layout and released hoisted
  bundle), so one snapshot serves both (files-snapshot-status.txt is now
  logical ids, 1,645 entries from the released linux-x64 bundle)
- still catches real regressions (verified: an injected module is reported
  by name; measured logical baselines 1625-1645 vs threshold 1700)

Every run also prints a physical/logical/duplication report line so CI logs
accumulate true per-environment baselines - the threshold tightens to
measured+~30 once those land, and duplication trends stay visible without
failing anyone. Bootstrap guard unchanged (stable, its own physical snapshot);
migrate it the same way once its CI report lines accumulate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant