Duplicate Code Detected: Cross-field invalidation logic in ModularChangeFamily managers

🔍 Duplicate Code Detected: Cross-field invalidation logic in ModularChangeFamily managers

Analysis of commit 618a6185c49462df696cb40a027948018bd720e7

Assignee: @copilot

Summary

ComposeManager.set() and RebaseManager.set() in modularChangeFamily.ts contain a large, near-identical block of logic for resolving cross-field keys and invalidating dependent fields. This duplication is in a complex/central area of the modular change family implementation, increasing the risk of future bugfixes diverging between the two code paths.

Duplication Details

Pattern: Cross-field-key → field-id resolution + dependent invalidation prior to super.set(...)

• Severity: Medium
• Occurrences: 2 (near-identical blocks >10 lines)
• Locations:
  • packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts (lines 2466–2514) — RebaseManager.set(...)
  • packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts (lines 2549–2601) — ComposeManager.set(...)

Code sample (excerpt showing the repeated shape)

public override set(...): void {
  if (invalidateDependents && this.allowInval) {
    const newFieldIds = getFieldsForCrossFieldKey(this.table.newChange, { target, revision, localId: id }, count);

    // RebaseManager: asserts newFieldIds.length === 0
    // ComposeManager: if (newFieldIds.length > 0) { ... } else { ... }

    const baseFieldIds = getFieldsForCrossFieldKey(this.table.baseChange, { target, revision, localId: id }, count);
    assert(baseFieldIds.length > 0, /* ... */);

    for (const baseFieldId of baseFieldIds) {
      // RebaseManager: this.table.affectedBaseFields.set(...)
      // ComposeManager: this.table.pendingCompositions.affectedBaseFields.set(...)
    }
  }

  super.set(target, revision, id, count, newValue, invalidateDependents);
}

Impact Analysis

• Maintainability: The duplicated logic is non-trivial and easy to subtly change incorrectly in one place but not the other.
• Bug Risk: Divergent behavior (especially around newFieldIds handling and base/new resolution) could lead to inconsistent invalidation semantics between compose vs rebase.
• Code Bloat: Adds repeated complexity within an already large file.

Refactoring Recommendations

1. Extract a shared helper for invalidation bookkeeping

   • Example: a private function near these classes that encapsulates:
     • getFieldsForCrossFieldKey(...) lookup(s)
     • base/new fallback/validation
     • updating the relevant “affected fields” set(s)
   • Estimated effort: Medium (2–6 hours)
   • Benefits: single point of truth, reduced risk of divergence

2. Parameterize the differences between compose vs rebase

   • Pass callbacks/targets for where to record affected fields (e.g., recordAffectedBaseField(fieldId) / recordAffectedNewField(fieldId))
   • Keep per-manager policy differences (e.g., “rebase forbids modifying new cross-field keys”) outside the shared core.

3. Optional: factor the repeated private get table(): XTable pattern

   • Several managers have the same return this.crossFieldTable as XTable; accessor pattern; consider a common typed base or constructor field.

Implementation Checklist

• Review the duplicated regions and confirm desired shared semantics
• Extract helper + add parameters/callbacks for per-manager differences
• Update managers to call helper
• Run existing unit/typetests relevant to modular schema / change families

Analysis Metadata

• Analyzed Files: Focused review of top-churn changed .ts/.mjs/.cjs files in the commit (non-test/workflow); duplication found in modularChangeFamily.ts.
• Detection Method: Serena semantic symbol analysis + targeted excerpt review
• Commit: 618a6185c49462df696cb40a027948018bd720e7
• Analysis Date: 2026-03-18

Generated by Duplicate Code Detector · ◷

To install this agentic workflow, run

gh aw add github/gh-aw/.github/workflows/duplicate-code-detector.md@94662b1dee8ce96c876ba9f33b3ab8be32de82a4

Read the RebaseManager.set and ComposeManager.set regions in packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts, especially lines 2466–2514 and 2549–2601. Review their differing newFieldIds and affected-field handling, then run the relevant modular schema unit tests and typetests. Done means shared invalidation logic is extracted, both managers use it, and their distinct policies remain intact.