Skip to content

Commit 1a54428

Browse files
committed
Decouple attribution interface from Automerge internals
Consumer-facing interfaces now expose CharAttribution[] entries instead of the full AttributionMap (which carries Automerge-specific processedHeads/processedHistoryIndex bookkeeping). This enables swapping the attribution data source (e.g. to git blame) without changing any consumer code.
1 parent 3ab292a commit 1a54428

7 files changed

Lines changed: 58 additions & 61 deletions

File tree

claude-notes/plans/2026-04-13-q2-debug-node-attribution.md

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,21 @@ The AST already carries source location info (`s` field on each node, referencin
6363
- [x] `ReactAstDebugRenderer.tsx` — in `Node`, consume `NodeAttributionContext`, apply color + title tooltip
6464
- [x] All Phase 3 tests pass (green) — 3 tests
6565

66+
### Phase 4: Decouple Attribution Interface from Automerge Internals
67+
68+
Refactor so that the consumer-facing interface carries only source-agnostic `CharAttribution[]` entries, not the full Automerge-specific `AttributionMap` (which includes `processedHeads` and `processedHistoryIndex` bookkeeping). This enables swapping the attribution data source (e.g., to git blame) without changing any consumer code.
69+
70+
- [x] `getNodeAttribution()` takes `entries: CharAttribution[]` instead of `attributionMap: AttributionMap`
71+
- [x] `AttributionContext` shape: `entries: CharAttribution[]` instead of `attributionMap: AttributionMap`
72+
- [x] `UseAttributionResult` type: `entries: CharAttribution[]` instead of `attributionMap: AttributionMap`
73+
- [x] `useAttribution` hook internally retains `AttributionMap` for incremental updates but only surfaces `.entries` to consumers
74+
- [x] `Editor.tsx` context value: `entries: attribution.entries`
75+
- [x] `ReactAstDebugRenderer.tsx`: reads `attributionCtx.entries`
76+
- [x] Tests updated: service tests pass `CharAttribution[]` directly; hook tests assert on `.entries`
77+
- [x] All 453 tests pass, TypeScript type-checks cleanly
78+
79+
**Rationale**: The `processedHeads` and `processedHistoryIndex` fields are Automerge incremental-update bookkeeping — they only matter inside the hook, never to consumers. With this refactor, a future git blame provider only needs to produce `{ entries: CharAttribution[], byteToCharMap: number[] }` (the same hook return shape), and everything downstream works unchanged.
80+
6681
### Verification
6782

6883
- [x] `npm run build:all` passes from hub-client (after annotated-qmd fixes in `c6200953`)
@@ -254,7 +269,7 @@ export function updateAttributionMap(
254269
export function getNodeAttribution(
255270
sourceInfoId: number,
256271
reconstructor: SourceInfoReconstructor,
257-
attributionMap: AttributionMap,
272+
entries: CharAttribution[], // source-agnostic (Phase 4 refactor)
258273
byteToCharMap: number[],
259274
identities: Record<string, ActorIdentity>,
260275
): NodeAttribution | null;
@@ -284,7 +299,7 @@ const sourceContext: SourceContext = {
284299

285300
**IMPORTANT — `content` is NOT in the WASM output**: The Rust JSON writer (`crates/pampa/src/writers/json.rs:66-72`) serializes `FileEntryJson` with only `{ line_breaks?, name, total_length? }` — no `content` field. The `SourceInfoReconstructor` constructor **throws** if `file.content` is null/undefined (`ts-packages/annotated-qmd/src/source-map.ts:63-67`). Before constructing the reconstructor, the QMD source text must be injected into `astContext.files[0].content`. This text is the same Automerge document content that was passed to the WASM parser. The `Ast` component must receive this text (via `AttributionContext` or as a prop) to populate the field.
286301

287-
**Query function**: `getNodeAttribution(sourceInfoId, reconstructor, attributionMap, byteToCharMap, identities) -> NodeAttribution | null` — calls `reconstructor.getSourceLocation(sourceInfoId)` to get `{ fileId, start, end }` in top-level byte coordinates, converts to char range via `byteToCharMap`, finds the most recent `{ actor, time }` in that range, resolves identity.
302+
**Query function**: `getNodeAttribution(sourceInfoId, reconstructor, entries, byteToCharMap, identities) -> NodeAttribution | null` — calls `reconstructor.getSourceLocation(sourceInfoId)` to get `{ fileId, start, end }` in top-level byte coordinates, converts to char range via `byteToCharMap`, finds the most recent `{ actor, time }` in the `CharAttribution[]` entries for that range, resolves identity.
288303

289304
### Phase 2: React Hook + Context
290305

@@ -296,25 +311,27 @@ Hook `useAttribution(filePath, identities)` that:
296311
3. **Cancellation on file switch**: When `filePath` changes, the hook aborts the in-flight build (if any) via `abortController.abort()`, resets state to `null`, and starts a fresh build for the new file.
297312
4. **Cancellation on unmount**: Cleanup function aborts any in-flight build.
298313
5. **Fallback to full rebuild**: If `updateAttributionMap` detects history compaction (stored index exceeds history length) or `diff()` fails, it throws a `HistoryCompactedError`. The hook catches this and starts a new async `buildAttributionMap` (returning `null` in the interim until it completes).
299-
6. Returns `{ attributionMap, byteToCharMap }` or `null`
314+
6. Returns `{ entries, byteToCharMap }` or `null` (entries is `CharAttribution[]`, extracted from the internal `AttributionMap`)
300315

301316
**Ref stability**: The hook stores the `AttributionMap` in a `useRef` alongside the React state setter, so the debounced callback always operates on the latest map without needing the state value in its closure (avoids stale-closure bugs with rapid edits).
302317

303318
**Context definition** (in the same file or a shared types file):
304319
```typescript
305320
export const AttributionContext = createContext<{
306-
attributionMap: AttributionMap;
321+
entries: CharAttribution[]; // source-agnostic per-character attribution
307322
byteToCharMap: number[];
308323
identities: Record<string, ActorIdentity>;
309324
sourceText: string; // QMD text from Automerge doc, needed to populate astContext.files[].content
310325
} | null>(null);
311326
```
312327

328+
**Design note (Phase 4 refactor)**: The context deliberately exposes `entries: CharAttribution[]` instead of the full `AttributionMap`. The Automerge-specific bookkeeping (`processedHeads`, `processedHistoryIndex`) stays internal to the hook. This allows a future git blame provider to produce the same context shape without any consumer changes.
329+
313330
### Phase 3: Wire Context in Editor, Extract + Render in Ast
314331

315332
**Attribution data flows via context**, avoiding prop drilling through PreviewRouter, ReactPreview, and ReactRenderer:
316333

317-
- [ ] `Editor.tsx` — call `useAttribution(currentFile?.path, identities)`, wrap preview area with `AttributionContext.Provider` passing `{ attributionMap, byteToCharMap, identities, sourceText }` where `sourceText` is the current Automerge document content (already available as the text passed to the preview pipeline)
334+
- [ ] `Editor.tsx` — call `useAttribution(currentFile?.path, identities)`, wrap preview area with `AttributionContext.Provider` passing `{ entries, byteToCharMap, identities, sourceText }` where `sourceText` is the current Automerge document content (already available as the text passed to the preview pipeline)
318335

319336
The `astJson` string flowing through `ReactPreview → ReactRenderer → Ast` is already the full `RustQmdJson` — it contains `astContext` and per-node `s` fields at runtime. No upstream component needs to extract or thread this data.
320337

hub-client/src/components/Editor.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ export default function Editor({ project, files, fileContents, onDisconnect, onC
365365
const attributionEnabled = currentFormat === 'q2-debug' && attributionPref;
366366
const attribution = useAttribution(attributionEnabled ? (currentFile?.path ?? null) : null, displayContent);
367367
const attributionContextValue = attribution ? {
368-
attributionMap: attribution.attributionMap,
368+
entries: attribution.entries,
369369
byteToCharMap: attribution.byteToCharMap,
370370
identities: identities ?? {},
371371
sourceText: displayContent,

hub-client/src/components/render/ReactAstDebugRenderer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ const AstRenderer = ({ ast, onNavigateToDocument, setAst }: {
575575
const result = getNodeAttribution(
576576
sourceInfoId,
577577
reconstructor,
578-
attributionCtx.attributionMap,
578+
attributionCtx.entries,
579579
byteToChar,
580580
attributionCtx.identities,
581581
);

hub-client/src/hooks/useAttribution.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ describe('useAttribution', () => {
9595

9696
// Now should have the map
9797
expect(result.current).not.toBeNull();
98-
expect(result.current!.attributionMap).toBe(mockMap);
98+
expect(result.current!.entries).toBe(mockMap.entries);
9999
});
100100

101101
it('calls updateAttributionMap on sourceText change when map exists', async () => {
@@ -163,7 +163,7 @@ describe('useAttribution', () => {
163163
// Wait for rebuild
164164
await act(async () => {});
165165

166-
expect(result.current!.attributionMap).toBe(freshMap);
166+
expect(result.current!.entries).toBe(freshMap.entries);
167167
});
168168

169169
it('aborts in-flight build and starts fresh on filePath change', async () => {
@@ -203,7 +203,8 @@ describe('useAttribution', () => {
203203
await act(async () => {});
204204

205205
expect(result.current).not.toBeNull();
206-
expect(result.current!.attributionMap.processedHistoryIndex).toBe(2);
206+
expect(result.current).not.toBeNull();
207+
expect(result.current!.entries).toHaveLength(1);
207208
});
208209

209210
it('aborts in-flight build on unmount', async () => {

hub-client/src/hooks/useAttribution.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
buildByteToCharMap,
1414
HistoryCompactedError,
1515
} from '../services/attribution';
16-
import type { AttributionMap } from '../services/attribution';
16+
import type { AttributionMap, CharAttribution } from '../services/attribution';
1717
import { getFileHandle } from '../services/automergeSync';
1818
import type { ActorIdentity } from '../services/automergeSync';
1919

@@ -22,7 +22,7 @@ import type { ActorIdentity } from '../services/automergeSync';
2222
// ---------------------------------------------------------------------------
2323

2424
export const AttributionContext = createContext<{
25-
attributionMap: AttributionMap;
25+
entries: CharAttribution[];
2626
byteToCharMap: number[];
2727
identities: Record<string, ActorIdentity>;
2828
sourceText: string;
@@ -33,7 +33,7 @@ export const AttributionContext = createContext<{
3333
// ---------------------------------------------------------------------------
3434

3535
interface UseAttributionResult {
36-
attributionMap: AttributionMap;
36+
entries: CharAttribution[];
3737
byteToCharMap: number[];
3838
}
3939

@@ -89,7 +89,7 @@ export function useAttribution(
8989
if (map) {
9090
const byteToChar = buildByteToCharMap(sourceTextRef.current);
9191
mapRef.current = map;
92-
setResult({ attributionMap: map, byteToCharMap: byteToChar });
92+
setResult({ entries: map.entries, byteToCharMap: byteToChar });
9393
} else {
9494
mapRef.current = null;
9595
setResult(null);
@@ -137,7 +137,7 @@ export function useAttribution(
137137
const updatedMap = updateAttributionMap(mapRef.current, handle, 'text');
138138
const byteToChar = buildByteToCharMap(sourceText);
139139
mapRef.current = updatedMap;
140-
setResult({ attributionMap: updatedMap, byteToCharMap: byteToChar });
140+
setResult({ entries: updatedMap.entries, byteToCharMap: byteToChar });
141141
} catch (err) {
142142
if (err instanceof HistoryCompactedError) {
143143
// History was compacted — need a full rebuild

hub-client/src/services/attribution.test.ts

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -510,17 +510,13 @@ describe('getNodeAttribution', () => {
510510
}
511511

512512
it('resolves source info to node attribution with correct identity', () => {
513-
const map: AttributionMap = {
514-
entries: [
515-
{ actor: 'actor1', time: 1000 },
516-
{ actor: 'actor1', time: 1000 },
517-
{ actor: 'actor2', time: 2000 },
518-
{ actor: 'actor2', time: 2000 },
519-
{ actor: 'actor2', time: 2000 },
520-
],
521-
processedHeads: ['h2'],
522-
processedHistoryIndex: 2,
523-
};
513+
const entries: CharAttribution[] = [
514+
{ actor: 'actor1', time: 1000 },
515+
{ actor: 'actor1', time: 1000 },
516+
{ actor: 'actor2', time: 2000 },
517+
{ actor: 'actor2', time: 2000 },
518+
{ actor: 'actor2', time: 2000 },
519+
];
524520

525521
// Source info ID 5 → file 0, bytes 2-5 → chars 2-5 (ASCII)
526522
const reconstructor = createMockReconstructor({ fileId: 0, start: 2, end: 5 });
@@ -529,7 +525,7 @@ describe('getNodeAttribution', () => {
529525
actor2: { name: 'Bob', color: '#2196F3' },
530526
};
531527

532-
const result = getNodeAttribution(5, reconstructor as any, map, byteToCharMap, identities);
528+
const result = getNodeAttribution(5, reconstructor as any, entries, byteToCharMap, identities);
533529

534530
expect(result).not.toBeNull();
535531
expect(result!.actor).toBe('actor2');
@@ -539,36 +535,28 @@ describe('getNodeAttribution', () => {
539535
});
540536

541537
it('returns null for invalid source info ID (reconstructor throws)', () => {
542-
const map: AttributionMap = {
543-
entries: [{ actor: 'actor1', time: 1000 }],
544-
processedHeads: ['h1'],
545-
processedHistoryIndex: 1,
546-
};
538+
const entries: CharAttribution[] = [{ actor: 'actor1', time: 1000 }];
547539

548540
const reconstructor = createMockReconstructor(null);
549541
const byteToCharMap = [0];
550542
const identities: Record<string, ActorIdentity> = {};
551543

552-
const result = getNodeAttribution(999, reconstructor as any, map, byteToCharMap, identities);
544+
const result = getNodeAttribution(999, reconstructor as any, entries, byteToCharMap, identities);
553545

554546
expect(result).toBeNull();
555547
});
556548

557549
it('returns null when identities map is empty', () => {
558-
const map: AttributionMap = {
559-
entries: [
560-
{ actor: 'actor1', time: 1000 },
561-
{ actor: 'actor1', time: 1000 },
562-
],
563-
processedHeads: ['h1'],
564-
processedHistoryIndex: 1,
565-
};
550+
const entries: CharAttribution[] = [
551+
{ actor: 'actor1', time: 1000 },
552+
{ actor: 'actor1', time: 1000 },
553+
];
566554

567555
const reconstructor = createMockReconstructor({ fileId: 0, start: 0, end: 2 });
568556
const byteToCharMap = [0, 1, 2]; // 2 bytes + end boundary
569557
const identities: Record<string, ActorIdentity> = {};
570558

571-
const result = getNodeAttribution(0, reconstructor as any, map, byteToCharMap, identities);
559+
const result = getNodeAttribution(0, reconstructor as any, entries, byteToCharMap, identities);
572560

573561
// Should still return attribution even without identity — uses fallback
574562
// The actor is known, but identity may not have name/color
@@ -577,40 +565,32 @@ describe('getNodeAttribution', () => {
577565
expect(result!.actor).toBe('actor1');
578566
});
579567

580-
it('returns null when attribution map is null-like (empty entries for range)', () => {
581-
const map: AttributionMap = {
582-
entries: [], // empty
583-
processedHeads: ['h1'],
584-
processedHistoryIndex: 1,
585-
};
568+
it('returns null when entries array is empty', () => {
569+
const entries: CharAttribution[] = [];
586570

587571
const reconstructor = createMockReconstructor({ fileId: 0, start: 0, end: 5 });
588572
const byteToCharMap = [0, 1, 2, 3, 4];
589573
const identities: Record<string, ActorIdentity> = {};
590574

591-
const result = getNodeAttribution(0, reconstructor as any, map, byteToCharMap, identities);
575+
const result = getNodeAttribution(0, reconstructor as any, entries, byteToCharMap, identities);
592576

593577
expect(result).toBeNull();
594578
});
595579

596580
it('finds most recent attribution in the byte range', () => {
597-
const map: AttributionMap = {
598-
entries: [
599-
{ actor: 'actor1', time: 1000 },
600-
{ actor: 'actor2', time: 3000 }, // most recent
601-
{ actor: 'actor1', time: 2000 },
602-
],
603-
processedHeads: ['h3'],
604-
processedHistoryIndex: 3,
605-
};
581+
const entries: CharAttribution[] = [
582+
{ actor: 'actor1', time: 1000 },
583+
{ actor: 'actor2', time: 3000 }, // most recent
584+
{ actor: 'actor1', time: 2000 },
585+
];
606586

607587
const reconstructor = createMockReconstructor({ fileId: 0, start: 0, end: 3 });
608588
const byteToCharMap = [0, 1, 2, 3]; // 3 bytes + end boundary
609589
const identities: Record<string, ActorIdentity> = {
610590
actor2: { name: 'Bob', color: '#E91E63' },
611591
};
612592

613-
const result = getNodeAttribution(0, reconstructor as any, map, byteToCharMap, identities);
593+
const result = getNodeAttribution(0, reconstructor as any, entries, byteToCharMap, identities);
614594

615595
expect(result).not.toBeNull();
616596
// Most recent is actor2 at time 3000

hub-client/src/services/attribution.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ export function buildByteToCharMap(text: string): number[] {
318318
export function getNodeAttribution(
319319
sourceInfoId: number,
320320
reconstructor: SourceInfoReconstructor,
321-
attributionMap: AttributionMap,
321+
entries: CharAttribution[],
322322
byteToCharMap: number[],
323323
identities: Record<string, ActorIdentity>,
324324
): NodeAttribution | null {
@@ -337,7 +337,6 @@ export function getNodeAttribution(
337337
if (charStart === undefined || charEnd === undefined) return null;
338338

339339
// Find the most recent attribution in this range
340-
const entries = attributionMap.entries;
341340
if (entries.length === 0) return null;
342341

343342
// Clamp range to entries bounds

0 commit comments

Comments
 (0)