Duplicate code: CrossFieldManager wrapper classes in modularChangeFamily.ts

Analysis of commit bd657fbd726a0d55605ca4d8c2e643b3945abdca

Summary

packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts contains three very similar classes (InvertManager, RebaseManager, ComposeManager) that repeat the same structural “wrapper around CrossFieldManagerI” pattern (constructor boilerplate, onMoveIn, moveKey, and a typed table getter).

This is localized duplication, but it’s substantial (>10 lines per class) and repeated 3 times, which increases maintenance cost and the chance of inconsistent future edits.

Duplication Details

Pattern: *Manager extends CrossFieldManagerI(FieldChange)

• Severity: Medium
• Occurrences: 3
• Locations:
  • packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts (lines 2427–2453) — InvertManager
  • packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts (lines 2455–2535) — RebaseManager
  • packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts (lines 2538–2618) — ComposeManager

Example of repeated structure

Each class:

• stores a fieldId
• calls super(table, currentField, allowInval)
• provides onMoveIn(...) that writes fieldId into a table-specific *NodeToParent map
• provides a typed private get table(): (TableType) which casts this.crossFieldTable

Code sample (excerpted from InvertManager):

class InvertManager extends CrossFieldManagerI(FieldChange) {
	public constructor(
		table: InvertTable,
		field: FieldChange,
		private readonly fieldId: FieldId,
		allowInval = true,
	) {
		super(table, field, allowInval);
	}

	public override onMoveIn(id: ChangeAtomId): void {
		setInChangeAtomIdMap(this.table.invertedNodeToParent, id, this.fieldId);
	}

	private get table(): InvertTable {
		return this.crossFieldTable as InvertTable;
	}
}

Impact Analysis

• Maintainability: Any future enhancement to this “manager wrapper” pattern likely requires editing 3 places.
• Bug Risk: Divergence risk (e.g., one manager updated to handle a new edge case while others are missed).
• Code Bloat: The repeated boilerplate obscures the real differences between the managers (how moveKey/set behave).

Refactoring Recommendations

1. Extract a typed base class to remove boilerplate

   • Create something like:
     • abstract class FieldIdCrossFieldManager(TTable extends CrossFieldTable<FieldChange)> extends CrossFieldManagerI(FieldChange)
     • store protected readonly fieldId: FieldId
     • provide protected get table(): TTable (typed cast once)
   • Then each manager only implements the truly distinct bits (set override, moveKey policy, and which map to update in onMoveIn).

2. Parameterize onMoveIn map selection

   • Option A: base helper protected onMoveInTo(map: ChangeAtomIdMap(FieldId), id: ChangeAtomId)
   • Option B: pass a getNodeToParentMap(table) callback to the base class.

Implementation Checklist

• Decide whether to use a shared base class vs. a small factory/helper function
• Refactor InvertManager, RebaseManager, ComposeManager to reuse shared implementation
• Ensure no behavioral changes (especially around invalidation behavior and moveKey semantics)
• Run existing unit/integration tests for the tree modular change family

Analysis Metadata

• Analyzed Files: 6 (top-churn non-test TS files sampled)
• Detection Method: Serena semantic code analysis (symbol inspection + pattern search)
• Commit: bd657fbd726a0d55605ca4d8c2e643b3945abdca
• Analysis Date: 2026-03-27

Generated by Duplicate Code Detector · ◷

To install this agentic workflow, run

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

Start in packages/dds/tree/src/feature-libraries/modular-schema/modularChangeFamily.ts by reading CrossFieldManagerI and the InvertManager, RebaseManager, and ComposeManager classes. Decide between the proposed shared base class and helper approach, then verify that invalidation, moveKey behavior, and each manager's node-to-parent map remain unchanged. Run the existing unit and integration tests for the tree modular change family when the refactor is complete.