Duplicate code: MutableStringInterner duplicated in framework and experimental tree

[github-actions]

🔍 Duplicate Code Detected: MutableStringInterner

Analysis of commit edf996f

Assignee: @copilot

Summary

MutableStringInterner (and its supporting StringInterner interface shape) is implemented twice in different packages with near-identical logic, creating ongoing maintenance risk and inconsistent behavior (notably differing error semantics in getString).

Duplication Details

Pattern: Near-identical MutableStringInterner implementation

• Severity: Medium

• Occurrences: 2

• Locations:

  • packages/framework/attributor/src/stringInterner.ts (lines 31-89)
  • experimental/dds/tree/src/StringInterner.ts (lines 23-74)

• Code Sample (abridged, showing the duplicated core):

  export class MutableStringInterner implements StringInterner {
  	private readonly stringToInternedIdMap = new Map(string, InternedStringId)();
  	private readonly internedStrings: string[] = [];
  
  	constructor(inputStrings: readonly string[] = []) {
  		for (const value of inputStrings) {
  			this.getOrCreateInternedId(value);
  		}
  	}
  
  	public getOrCreateInternedId(input: string): InternedStringId {
  		return this.getInternedId(input) ?? this.createNewId(input);
  	}
  
  	public getInternedId(input: string): InternedStringId | undefined {
  		return this.stringToInternedIdMap.get(input);
  	}
  
  	public getString(internId: number): string {
  		// differs only in the error behavior (UsageError vs fail)
  		/* ... */
  	}
  
  	public getSerializable(): readonly string[] {
  		return this.internedStrings;
  	}
  
  	private createNewId(input: string): InternedStringId {
  		const internedId = this.stringToInternedIdMap.size as InternedStringId;
  		this.stringToInternedIdMap.set(input, internedId);
  		this.internedStrings.push(input);
  		return internedId;
  	}
  }

Impact Analysis

• Maintainability: Fixes/improvements must be applied twice; comments and error-handling have already diverged.
• Bug Risk: Different failure behavior (UsageError throw vs fail(...)) can cause subtle differences across call sites.
• Code Bloat: ~40-50 LOC duplicated across two packages, plus duplicated interface definitions.

Refactoring Recommendations

1. Choose a single canonical implementation

   • Option A: Move the implementation to a small shared internal utility module (e.g. packages/common/* or an existing lightweight shared package) and import it from both locations.
   • Option B: If the experimental tree implementation can depend on the framework version (or vice-versa), re-export/use the existing one and delete the duplicate.
   • Benefits: Single source of truth; consistent semantics.

2. Align error semantics

   • Decide whether out-of-range IDs should throw UsageError or use fail(...), then standardize.
   • Benefits: Consistent behavior and diagnostics across packages.

Implementation Checklist

• Confirm intended dependency direction between packages/framework/attributor and experimental/dds/tree
• Pick a shared location for the canonical implementation
• Update both call sites to import/re-export
• Standardize getString failure semantics
• Run relevant package builds/tests

Analysis Metadata

• Analyzed Files: 2 (targeted semantic search based on getString/StringInterner hits)
• Detection Method: Serena semantic code analysis
• Commit: edf996f
• Analysis Date: 2026-03-29T21:50:29Z

Generated by Duplicate Code Detector · ◷

To install this agentic workflow, run

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