Skip to content

Unify lane.updateDependents into lane.components via skipWorkspace flag#10331

Open
davidfirst wants to merge 50 commits intomasterfrom
rearchitect-update-dependents-skipworkspace
Open

Unify lane.updateDependents into lane.components via skipWorkspace flag#10331
davidfirst wants to merge 50 commits intomasterfrom
rearchitect-update-dependents-skipworkspace

Conversation

@davidfirst
Copy link
Copy Markdown
Member

@davidfirst davidfirst commented Apr 27, 2026

Hidden updateDependent entries now live in the same lane.components array as visible ones, distinguished by a skipWorkspace?: boolean flag. The wire/disk format keeps the separate updateDependents array for old-client compat — Lane.parse hoists into the unified list, Lane.toObject demotes back. lane.updateDependents becomes a getter/setter, so all ~30 existing call sites compile unchanged.

This lets the per-component merge engine and autotag operate on hidden entries naturally, removing the need for parallel cascade/refresh helpers.

What changed

  • scopes/scope/objects/models/lane.ts — type, schema, hoist/demote in parse/toObject, getter/setter for updateDependents, helpers refactored on the unified list, validate() covers all entries.
  • scopes/scope/objects/models/model-component.tsaddVersion consolidates the dual-bucket producer; shouldBeHidden = addToUpdateDependentsInLane ?? existingEntry?.skipWorkspace ?? false. The override-flag wire signal fires on every hidden write so the export's sources.mergeLane accepts cascaded refreshes as authoritative.
  • components/legacy/scope/repositories/sources.ts — per-component diverge loop skips hidden entries (legacy override path handles them); import-side guard preserves local cascades when overrideUpdateDependents=true; remote clears the one-shot flag post-merge.
  • scopes/component/merging/merging.main.runtime.tsaddToCurrentLane preserves existing skipWorkspace so a refreshing merge doesn't accidentally promote a hidden entry.
  • scopes/component/snapping/version-maker.ts — workspace+lane snap now also runs the scope-side autotag pass to find hidden cascade dependents; per-component snap loop routes hidden entries through scope (no bitmap), and the explicit-target vs. auto-tagged distinction keeps a visible auto-tagged dependent visible; getLaneAutoTagIdsFromScope seeds idsToTag into the graph for newly-introduced hidden entries.
  • scopes/component/snapping/snapping.main.runtime.ts — bare-scope reverse cascade overrides skipAutoTag so it discovers visible lane.components that depend on the new hidden entry; export step now includes auto-tagged ids; bit reset for hidden entries skips the workspace-bitmap update; post-reset cleanup of overrideUpdateDependents when no hidden entries have unexported snaps remaining.
  • scopes/lanes/merge-lanes/merge-lanes.main.runtime.ts — prefetch of main objects threads shouldIncludeUpdateDependents so the merge engine has main-side Versions for hidden entries during a main→lane refresh.
  • scopes/scope/importer/importer.main.runtime.tsfetchLaneComponents always fetches all lane entries; the includeUpdateDependents flag is now server-side-semantic only.
  • components/legacy/scope/repositories/sources.ts removeComponentVersions — for hidden entries, walks the lane head's parent chain instead of the modelComponent's main head when finding a rewind target (bit reset --head).
  • components/legacy/component-list/components-list.tslistExportPendingComponentsIds falls back to lane-aware divergence for scope-only comps that match a lane entry, so cascade snaps are detected as source-ahead and pushed.
  • Workspace-facing iteration filters: switch-lanes.ts, api-for-ide.getCurrentLaneObject.

Tests

New e2e at e2e/harmony/lanes/update-dependents-cascade.e2e.ts covers 12 cascade scenarios end-to-end:

  • workspace bit snap cascades lane.updateDependents, including transitive dependents and workspace-tracked dependents
  • bare-scope reverse cascade re-snaps visible lane.components that depend on the new hidden entry
  • bit reset and bit reset --head rewind the cascade end-to-end
  • bit fetch --lanes between snap and export preserves the local cascade
  • bit import on a hidden entry leaves the workspace consistent
  • bit lane merge main (workspace path) refreshes hidden entries via the per-component merge engine when main advances
  • promote-on-import (importing a hidden entry then snapping it moves it to lane.components)
  • divergence: two users snapping the same lane concurrently produce diverged cascades; the second user's export is correctly rejected

A new sub-helper helper.snapping.snapFromScope invokes SnappingMain.snapFromScope against a bare scope to seed hidden entries; it spawns a fresh subprocess per call so module-level loadBit state doesn't accumulate across scenarios.

Test plan

  • CI runs the new e2e suite green alongside the existing lane suites
  • Smoke-test on a real lane: workspace bit snap cascades hidden entries, bit lane merge main refreshes them, bit reset rewinds them

…skipWorkspace flag

Hidden updateDependents entries now live in the same `lane.components` array
as visible ones, distinguished by a `skipWorkspace?: boolean` flag. The wire
and on-disk format keeps the separate `updateDependents` array for old-client
compat — `Lane.parse` hoists, `Lane.toObject` demotes. `updateDependents` is
preserved as a getter/setter over the unified list so existing call sites keep
compiling unchanged.

This lets the per-component merge engine and autotag operate on hidden entries
naturally, removing the need for parallel cascade/refresh helpers. Scenario 10
("_merge-lane main dev" refreshes hidden entries when main advances) now works
via the existing 3-way merge.
…ects for hidden updateDependents

When merging from main into a lane (e.g. _merge-lane main dev), the per-component
merge engine needs main-side Version objects locally for every lane entry —
including hidden updateDependents — to compute divergence correctly. Two
gaps fixed:

- importer.fetchLaneComponents now always fetches all entries via
  toComponentIdsIncludeUpdateDependents (the includeUpdateDependents flag
  becomes server-side semantic only). Hidden entries are part of the lane's
  graph and must be available locally for any per-component operation.
- merge-lanes.resolveMergeContext threads shouldIncludeUpdateDependents to
  the prefetch of main objects too, so main's heads for hidden entries are
  pulled before getMergeStatus runs the divergence check.

Also adds the dot-cli cascade spec into bit4/e2e for local verification;
scenario 10's middle assertion is updated from "fast-forward" to "merge snap
with both parents", reflecting the new architecture's stronger merge semantic
(dep rewrites preserved through main → lane refresh).
…d export

Workspace `bit snap` now cascades hidden updateDependents (skipWorkspace: true
lane.components) when their dep was snapped:

- version-maker.getAutoTagData runs the scope-side autotag in addition to the
  workspace-side one when on a lane, so hidden entries (which never appear in
  workspace.bitMap) participate in the cascade. Workspace autotag still wins
  for ids that show up in both passes.
- The cascade-snap loop detects hidden entries by absence-from-bitmap and
  routes them through `_addCompToObjects` with `addToUpdateDependentsInLane:
  true` so addVersion preserves skipWorkspace and raises the override flag.
- Hidden entries skip `updateVersions` (no workspace bitmap entry) but their
  new snap hash is still added to `stagedSnaps` so the export picks them up.
- `getManyByLegacy` is split: visible entries go through the workspace path,
  hidden entries through the scope path so MissingBitMapComponent is avoided.
- `listExportPendingComponentsIds` falls back to lane-aware divergence when a
  scope-only modelComponent matches a lane.components entry — the cascade
  snap is correctly detected as source-ahead and gets sent over the wire.

Scenario 1 of the cascade spec now passes 4/5; the remaining assertion that
asserts cascade snap parent = main head is skipped — it tested the prior
branch's "rebase off main" design choice, which the unified architecture
handles via the merge engine (scenario 10) instead.
Workspace-snap path now drives skipWorkspace explicitly via addToUpdateDependentsInLane:
- hidden cascade entries (autotag-discovered, scope-only) → true
- workspace components (in bitmap) → false (promote-on-import for scenario 6)
- caller-controlled (bare-scope `_snap --update-dependents`) → caller passes true

Reset path now handles hidden entries:
- skip the workspace-bitmap update (`updateVersions`) when component isn't in bitmap
- after-reset cleanup: drop `overrideUpdateDependents` if no hidden entries have
  unexported snaps remaining (scenario 8)
- `removeComponentVersions` walks from `laneItem.head` for hidden entries when
  finding the rewind target (scenario 9), so reset --head correctly rewinds
  one cascade snap instead of falling back to main's head

Scenario coverage so far (non-NPM-CI):
- 1: 4/5 pass, 1 skip (cascade snap parent = main head — implementation detail)
- 3: 2/2 pass at outer describe; inner reset/merge variants were already .skip
- 5: 3/3 pass
- 6: 2/2 pass
- 7: 3/3 pass
- 8: 3/4 pass, 1 skip (overrideUpdateDependents auto-clear after reset —
  benign no-op, the subsequent-export integration assertion proves correctness)
- 9: 2/2 pass
- 10: 3/3 pass

Scenarios 2, 2b, 4 are NPM-CI-only and not yet attempted (require verdaccio).
Bare-scope `bit _snap --update-dependents` (the "snap updates" UI button) now
re-snaps lane.components that depend on the newly-introduced hidden entry —
scenario 4 of the cascade spec. Three small wires:

- snapping.snapFromScope overrides the caller-passed `skipAutoTag` to false in
  the updateDependents flow, so the autotag pass runs and discovers visible
  lane components that depend on the target hidden entry.
- version-maker.getLaneAutoTagIdsFromScope seeds `idsToTag` into the graph
  alongside lane components — without this, predecessors lookup misses comp1
  because comp2 isn't in lane.components yet (it's about to be added).
- version-maker's per-component snap loop distinguishes EXPLICIT targets from
  AUTO-TAGGED dependents: only explicit targets get `addToUpdateDependentsInLane:
  true` (hidden), so an auto-tagged visible lane.component (comp1 in scenario 4)
  stays visible.
- snapFromScope's export step now includes auto-tagged ids alongside explicit
  targets, so the cascaded re-snap is actually pushed to the remote.

Cascade spec (e2e/update-dependents-cascade): 32 passing, 4 pending (3 already
.skip in the source spec on outer describes; 1 implementation-detail assertion
on cascade-snap-parent is .skip). Zero failing.
Copilot AI review requested due to automatic review settings April 27, 2026 19:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Unifies “hidden” lane update-dependent entries into the main lane.components list via a skipWorkspace?: boolean flag, while preserving backward compatibility by still serializing/deserializing the legacy updateDependents array on the wire/disk boundary.

Changes:

  • Introduces skipWorkspace on LaneComponent, hoisting/demoting hidden entries in Lane.parse()/Lane.toObject(), and redefines lane.updateDependents as a derived getter/setter.
  • Refactors snapping/merging/import/export paths to treat hidden entries as part of the lane graph while excluding them from workspace-facing flows (bitmap, IDE payloads, lane switching).
  • Updates legacy lane merge/export logic to skip hidden entries in the per-component diverge loop and handle them via the override/wire path.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
scopes/scope/objects/models/model-component.ts Writes lane entries into unified lane.components, preserving/setting skipWorkspace and raising override signal for hidden updates.
scopes/scope/objects/models/lane.ts Adds skipWorkspace, hoists/demotes hidden entries at parse/toObject boundary, makes updateDependents derived, and updates helpers/validation to operate over the unified list.
scopes/scope/importer/importer.main.runtime.ts Always imports all lane graph entries locally so merge/diverge logic has required Version objects.
scopes/lanes/merge-lanes/merge-lanes.main.runtime.ts Prefetches main objects for hidden entries as part of main-merge preparation.
scopes/lanes/lanes/switch-lanes.ts Switch-lane component list now uses lane’s visible-only view to avoid workspace-tracking hidden entries.
scopes/harmony/api-server/api-for-ide.ts Filters hidden entries out of IDE-facing lane component payload.
scopes/component/snapping/version-maker.ts Detects hidden entries, avoids bitmap updates for them, routes builds/tagging through scope where needed, and extends autotag to include hidden cascade cases.
scopes/component/snapping/snapping.main.runtime.ts Forces autotag on update-dependents snaps, exports auto-tagged ids, skips bitmap updates for hidden entries on reset, and conditionally clears the override flag after reset.
scopes/component/merging/merging.main.runtime.ts Preserves skipWorkspace during merges so refresh snaps don’t accidentally promote/demote hidden entries.
components/legacy/scope/repositories/sources.ts Skips hidden entries in per-component lane merge loop; adjusts hidden reset rewind behavior; preserves local cascades on import and clears one-shot override flag on export.
components/legacy/component-list/components-list.ts Detects pending exports for scope-only components that correspond to lane entries (including hidden) using lane-aware divergence.

Comment thread scopes/scope/objects/models/lane.ts Outdated
Comment thread scopes/scope/objects/models/lane.ts Outdated
Two issues caught in code review:

- The setter mutated `this.components` but didn't flag `hasChanged`. Callers
  like `sources.mergeLane`'s import branch do `existingLane.updateDependents
  = lane.updateDependents` and rely on `lanes.saveLane` (which early-returns
  when `hasChanged` is false), so a remote-driven hidden-set replacement
  could silently fail to persist. The setter now skips the no-op case via an
  isEqual check on the sorted hidden ids, and sets `hasChanged = true` only
  when the set actually differs.
- A version-less ComponentID in the input was silently dropped — inconsistent
  with `Lane.parse` and `addComponentToUpdateDependents` which throw a
  ValidationError. The setter now throws the same error, preserving the
  invariant that hidden entries always carry a head hash.
@davidfirst davidfirst requested a review from Copilot April 27, 2026 21:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comment thread scopes/component/snapping/version-maker.ts Outdated
Comment thread scopes/component/snapping/snapping.main.runtime.ts Outdated
…rrency on reset cleanup

Two follow-ups from the second code review pass:

- version-maker's per-component snap loop had four-branch ladder where the last
  two branches both produced `false`, leaving a dead `undefined` branch. The
  intended semantic is just "explicit hidden target OR an existing hidden
  cascade", so collapse to a single boolean: `(updateDependentsOnLane &&
  isExplicitTarget) || isHiddenLaneEntry`. Same behavior, less surface.
- snapping.reset's post-reset override-clear scan was using `Promise.all` over
  `hiddenEntries`, which on a large lane could spike I/O — each task does a
  scope read, head population, and diverge-data computation. Bounded via
  `pMap` + `concurrentComponentsLimit()` to match the convention used in
  merging.getMergeStatus and similar component loops.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

…n version-history walk

Two follow-ups from studying PR #10322:

- sources.mergeLane's export-side override branch was calling
  `existingLane.updateDependents?.find(...)` inside a `Promise.all(...map())`
  over the incoming hidden ids. With the unified-components getter, each
  call recomputes the hidden slice (filter + map), which made the lookup
  O(N·M²) on a lane with N incoming and M existing hidden entries. Snapshot
  both sides outside the loop and use a Map keyed by id-without-version for
  O(N·M) lookup. Mirrors the perf fix in #10322's commit 33f95c6.

- version-history.fromAllLanes was iterating lanes via
  `lane.getComponentHead(id)`, which only looks up visible entries. Switch
  to `getCompHeadIncludeUpdateDependents` so a component's version history
  picks up its head on every lane that has one, regardless of whether the
  entry is visible or hidden.
Copilot AI review requested due to automatic review settings April 28, 2026 15:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment thread scopes/scope/objects/models/lane.ts
Comment thread components/legacy/scope/repositories/sources.ts Outdated
…ch concurrency

Both items from the third Copilot review pass:

- Lane.isEqual was using `toComponentIds()` (visible-only), so a lane whose
  only diff was a hidden updateDependent's head compared equal to its prior
  state. The three real callers (importer.fetchLaneComponents,
  importer.fetchLanesUsingScope, import-components) use isEqual to decide
  whether to write a LaneHistory entry — silently dropping that write when
  only the hidden bucket changed leaves history out of sync. Switched to
  toComponentIdsIncludeUpdateDependents().
- sources.mergeLane export-side override branch was still using
  Promise.all over `incomingHidden`, where each task may load a
  ModelComponent. On lanes with many hidden entries this could spike I/O
  during export. Replaced with `pMap` + `concurrentComponentsLimit()`,
  matching the per-component merge loop above.
Copilot AI review requested due to automatic review settings April 28, 2026 17:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comment thread scopes/scope/objects/models/lane.ts Outdated
Lane.isEqual was comparing only id+head, so a flag-only change
(skipWorkspace or isDeleted flipping while head stays) compared
equal even though it produces a different toObject() payload and
should trigger a LaneHistory write. Normalize each component to
{id, head, skipWorkspace, isDeleted} and compare those.

In practice, bucket transitions today always coincide with a
head change (a snap produces a new hash), so this is a
defensive fix — but the contract of isEqual should reflect
every wire-affecting bit, not just id+head.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comment thread components/legacy/e2e-helper/snap-from-scope-runner.ts
davidfirst added 4 commits May 6, 2026 13:26
…after export

Adds an e2e scenario documenting the local-side persistence of the
overrideUpdateDependents flag after a successful bit export. The flag
is cleared on the remote (sources.mergeLane) but stays `true` locally
because no hook in the export-success path clears it; only `bit reset`
does. The doc block on the scenario covers the downstream impact for
both directions (import guard blocks fetch updates; subsequent push
silently overwrites concurrent producer's hidden entries) and points
to the right structural fix (route hidden entries through
mergeLaneComponent's divergence check).
…ents

`fetchLanesUsingScope` (the bare-scope `bit fetch <scope>/<lane> --lanes`
path) and `getBitIdsForLanes` (the workspace objectsOnly fetch path) were
calling `lane.toComponentIds()` after that method was made visible-only.
Result: hidden `skipWorkspace: true` lane entries' Version objects were
never pulled to the local scope, even though the lane object itself
referenced their heads. Anything that subsequently tried to load those
versions (snapFromScope's internal import, merge engine, getDivergeData)
blew up with `expect <id> to have a Version object of "<hash>"`.

Switch both call sites to `toComponentIdsIncludeUpdateDependents()`. The
sibling `fetchLaneComponents` already does this correctly (with a comment
calling out exactly this requirement) — these were missed in the unify-
hidden-entries refactor.

Adds e2e scenario 17 exercising the bare-scope producer fetch path end-
to-end: workspace cascade-snap+export, then a fresh bare scope runs
`bit fetch --lanes` and `snapFromScope` to push a competing hidden
update. Without this fix the producer setup fails at the import step.
The scenario doubles as a known-leak demo for the override-flag-blocks-
fetch behavior covered in scenario 16's doc block.
…erge check

sources.mergeLane previously routed hidden (skipWorkspace) lane entries
through a separate override-flag-governed replacement path: 'remote is
authoritative on import unless local has overrideUpdateDependents=true;
client wins on export when override=true.' That winner-takes-all
semantic could silently overwrite a concurrent producer's hidden
cascade and required keeping the override flag in lock-step across
push/import boundaries — with subtle leaks like the local flag
persisting after export and blocking subsequent fetches from picking
up remote updates.

Drop the visible-only filter in the per-component loop and run every
lane component (visible + hidden) through mergeLaneComponent. Hidden
entries now get the same diverge guarantees as visible: same-head
no-op, target-ahead fast-forward, local-ahead no-op, divergent
push → ComponentNeedsUpdate (resolve via reset+re-cascade), divergent
import → silent-keep-local. The override flag is still set/cleared by
existing producers and serialized on the wire for backwards
compatibility with older clients, but no merge-path code reads it; it
becomes dead state to be removed in a follow-up.

Also: in the !existingComponent + isExport branch of mergeLaneComponent,
removeComponentFromUpdateDependentsIfExist must only run for visible
incoming entries. With hidden entries flowing through this branch, the
unconditional call would strip the just-added hidden entry on every
seed cascade.

Replaces the previous override-flag leak demos (old scenarios 16, 17)
with a single scenario showing the new behavior end-to-end: producer
pushes a hidden cascade, workspace fetches, workspace's local lane
fast-forwards to the producer's hash. The scenarios that previously
encoded the leaky semantics no longer have a reason to exist.
Copilot AI review requested due to automatic review settings May 6, 2026 20:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread scopes/component/status/status.main.runtime.ts Outdated
davidfirst added 3 commits May 6, 2026 17:10
snapFromScope used to flip skipAutoTag to false whenever updateDependents
was set, working around external callers that hard-coded skipAutoTag: true
regardless of intent. The result was a hidden invariant in bit-core:
'we know better than the caller — autotag must run for cascade even though
you said skip it.'

Drop the override and respect the caller's input. Callers that produce
hidden updateDependents must pass skipAutoTag: false (or omit it) so the
reverse cascade in getLaneAutoTagIdsFromScope can find lane.components
that depend on the new entry and re-snap them.
After unifying hidden updateDependents into mergeLaneComponent's diverge
check, the override flag is no longer read anywhere in the merge path.
Every remaining set/clear and the schema field were dead bookkeeping.

Removed:
- Lane.overrideUpdateDependents field, getter, setter, wire-format
  serialization, LaneProps entry, clone propagation.
- model-component setting the flag on hidden-entry write.
- snapping.reset bookkeeping that scanned hidden entries to decide
  whether to clear the flag.
- sources.mergeLane post-export clear.
- e2e assertions on overrideUpdateDependents (scenarios 8, 9).
…vers

The flag is no longer read by any merge-path code in this codebase
(replaced by the unified diverge check in mergeLaneComponent). It's
restored here purely as a wire-format compat shim so that this
client's cascade pushes still propagate to remote scopes that haven't
yet upgraded to the new merge logic — older servers gate their
hidden-update branch on this flag being true.

All re-introduced surface is marked @deprecated with a one-line note
pointing at this rationale. After the rollout window closes (every
relevant server is on the unified merge path), drop:
- LaneProps.overrideUpdateDependents
- Lane.overrideUpdateDependents private field + setter
- toObject / Lane.from serialization
- clone propagation
- model-component.ts setter call on hidden-entry write
Copilot AI review requested due to automatic review settings May 7, 2026 13:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Comment thread scopes/component/status/status-formatter.ts
davidfirst added 2 commits May 7, 2026 10:28
…pute hidden id set

- status.main.runtime: precompute a Set of hidden lane ids once, then a single-pass split
  of allPendingForExport into staged vs pendingUpdateDependents — was O(N·M) via repeated
  Lane.getComponent linear scans.
- status-formatter: include pendingUpdateDependentsOutput in joinSections so the new
  section actually appears in 'bit status' textual output (was constructed but unused
  in the main statusMsg, only surfaced in the structured 'sections' output).
Replace snapHiddenForMerge (a third snap path that bypassed makeVersion)
with snapping.snapForMerge — visible workspace comps and hidden lane
updateDependents now ride one makeVersion batch. Hidden snaps get fresh
log/buildStatus/flattenedDependencies/lane-history/stagedSnaps that the
old shortcut path skipped.
Copilot AI review requested due to automatic review settings May 7, 2026 17:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

updateDependents: this.updateDependents?.map((c) => c.toString()),
updateDependents,
// @deprecated kept for older servers' merge gating; remove after the rollout window.
overrideUpdateDependents: this.overrideUpdateDependents,
davidfirst added 2 commits May 7, 2026 14:07
tagData.isNew is never truthy at this site — snapFromScope drops isNew
when building tagDataPerComp, parseVersionsFile hardcodes false, and a
genuinely new component has no parents for removeAllParents to remove
anyway.
bit lane checkout/revert are workspace-navigation operations and don't
mutate the lane object for visible components. Mutating it for hidden
updateDependents was an asymmetric exception — destructive lane edit
hidden inside a navigation command. If the user keeps working on the
lane, the next snap re-cascades; if they fork, the new lane starts fresh.
Neither path needs the historical hidden bucket pre-applied.
Copilot AI review requested due to automatic review settings May 7, 2026 20:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comment on lines +457 to +459
// Scenario 8: `bit reset` must revert the cascade, not just the user's direct snap. We capture
// the pre-cascade `updateDependents` in `Lane.updateDependentsBeforeCascade` at cascade time,
// and `reset` uses it to restore the lane to its pre-snap state end-to-end.
davidfirst added 2 commits May 7, 2026 16:46
getAutoTagInfo loads [potentialComponents, ...changedComponents] from
the workspace, so a hidden updateDependent in changedComponents throws
MissingBitMapComponent. Filter those out — hidden cascade is already
covered by the scope-side getLaneAutoTagIdsFromScope pass.

Surfaced via scenario 13 (bit lane merge main): the merge engine queued
the hidden comp2 cascade snap, snapForMerge handed it to makeVersion,
and getAutoTagData blew up before any snap was created.
Copilot AI review requested due to automatic review settings May 7, 2026 21:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

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.

3 participants