F3 SLICE-3/4 review fixes: counted self-writes, persist deletion-to-empty, render all visible editors (#6)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -28,7 +28,7 @@
|
|||||||
{ "command": "cowriting.reply", "title": "Reply", "category": "Cowriting" },
|
{ "command": "cowriting.reply", "title": "Reply", "category": "Cowriting" },
|
||||||
{ "command": "cowriting.resolveThread", "title": "Resolve Thread", "category": "Cowriting" },
|
{ "command": "cowriting.resolveThread", "title": "Resolve Thread", "category": "Cowriting" },
|
||||||
{ "command": "cowriting.reopenThread", "title": "Reopen Thread", "category": "Cowriting" },
|
{ "command": "cowriting.reopenThread", "title": "Reopen Thread", "category": "Cowriting" },
|
||||||
{ "command": "cowriting.toggleAttribution", "title": "Cowriting: Toggle Attribution", "category": "Cowriting" },
|
{ "command": "cowriting.toggleAttribution", "title": "Toggle Attribution", "category": "Cowriting" },
|
||||||
{ "command": "cowriting.applyAgentEdit", "title": "Apply Agent Edit (internal seam)", "category": "Cowriting" }
|
{ "command": "cowriting.applyAgentEdit", "title": "Apply Agent Edit (internal seam)", "category": "Cowriting" }
|
||||||
],
|
],
|
||||||
"menus": {
|
"menus": {
|
||||||
|
|||||||
@@ -31,6 +31,12 @@ interface DocAttribution {
|
|||||||
orphans: AttributionRecord[];
|
orphans: AttributionRecord[];
|
||||||
/** record metadata per live span id (updatedAt bookkeeping). */
|
/** record metadata per live span id (updatedAt bookkeeping). */
|
||||||
records: Map<string, AttributionRecord>;
|
records: Map<string, AttributionRecord>;
|
||||||
|
/**
|
||||||
|
* true once this doc has had any span/record this session — lets save persist
|
||||||
|
* deliberate deletion to empty (so stale records don't come back as phantom
|
||||||
|
* orphans on reload).
|
||||||
|
*/
|
||||||
|
hadAttributions: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
const AGENT_DECO: vscode.DecorationRenderOptions = {
|
const AGENT_DECO: vscode.DecorationRenderOptions = {
|
||||||
@@ -77,7 +83,7 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
private state(docPath: string): DocAttribution {
|
private state(docPath: string): DocAttribution {
|
||||||
let s = this.docs.get(docPath);
|
let s = this.docs.get(docPath);
|
||||||
if (!s) {
|
if (!s) {
|
||||||
s = { docPath, spans: [], orphans: [], records: new Map() };
|
s = { docPath, spans: [], orphans: [], records: new Map(), hadAttributions: false };
|
||||||
this.docs.set(docPath, s);
|
this.docs.set(docPath, s);
|
||||||
}
|
}
|
||||||
return s;
|
return s;
|
||||||
@@ -110,6 +116,7 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
s.spans = coalesce(s.spans);
|
s.spans = coalesce(s.spans);
|
||||||
|
if (artifact.attributions.length > 0) s.hadAttributions = true;
|
||||||
}
|
}
|
||||||
this.render(document);
|
this.render(document);
|
||||||
}
|
}
|
||||||
@@ -135,7 +142,9 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const s = this.state(docPath);
|
const s = this.state(docPath);
|
||||||
for (const change of e.contentChanges) {
|
// Sort descending by offset so earlier changes don't invalidate later offsets
|
||||||
|
// (VS Code's order is undocumented; defensive sort is the safe guarantee).
|
||||||
|
for (const change of [...e.contentChanges].sort((a, b) => b.rangeOffset - a.rangeOffset)) {
|
||||||
const edit = {
|
const edit = {
|
||||||
start: change.rangeOffset,
|
start: change.rangeOffset,
|
||||||
end: change.rangeOffset + change.rangeLength,
|
end: change.rangeOffset + change.rangeLength,
|
||||||
@@ -149,6 +158,7 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
turnId: hit?.turnId,
|
turnId: hit?.turnId,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
if (s.spans.length > 0) s.hadAttributions = true;
|
||||||
this.render(e.document);
|
this.render(e.document);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -193,7 +203,10 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
if (!this.isTracked(document)) return;
|
if (!this.isTracked(document)) return;
|
||||||
const docPath = this.docPathOf(document.uri);
|
const docPath = this.docPathOf(document.uri);
|
||||||
const s = this.docs.get(docPath);
|
const s = this.docs.get(docPath);
|
||||||
if (!s || (s.spans.length === 0 && s.orphans.length === 0)) return;
|
// Allow save when hadAttributions is true even if spans/orphans are now empty:
|
||||||
|
// that means the user deliberately deleted all attributed text, and we must
|
||||||
|
// persist a.attributions=[] so stale records don't return as phantom orphans.
|
||||||
|
if (!s || (!s.hadAttributions && s.spans.length === 0 && s.orphans.length === 0)) return;
|
||||||
const text = document.getText();
|
const text = document.getText();
|
||||||
const now = new Date().toISOString();
|
const now = new Date().toISOString();
|
||||||
const records: AttributionRecord[] = [];
|
const records: AttributionRecord[] = [];
|
||||||
@@ -239,14 +252,21 @@ export class AttributionController implements vscode.Disposable {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private render(document: vscode.TextDocument): void {
|
private render(document: vscode.TextDocument): void {
|
||||||
const editor = vscode.window.visibleTextEditors.find((e) => e.document === document);
|
if (!this.isTracked(document)) return;
|
||||||
if (!editor || !this.isTracked(document)) return;
|
|
||||||
const s = this.docs.get(this.docPathOf(document.uri));
|
const s = this.docs.get(this.docPathOf(document.uri));
|
||||||
const spans = this.visible && s ? s.spans : [];
|
const spans = this.visible && s ? s.spans : [];
|
||||||
const toRange = (sp: LiveSpan) =>
|
const toRange = (sp: LiveSpan) =>
|
||||||
new vscode.Range(document.positionAt(sp.start), document.positionAt(sp.end));
|
new vscode.Range(document.positionAt(sp.start), document.positionAt(sp.end));
|
||||||
editor.setDecorations(this.agentType, spans.filter((x) => x.author.kind === "agent").map(toRange));
|
const agentRanges = spans.filter((x) => x.author.kind === "agent").map(toRange);
|
||||||
editor.setDecorations(this.humanType, spans.filter((x) => x.author.kind === "human").map(toRange));
|
const humanRanges = spans.filter((x) => x.author.kind === "human").map(toRange);
|
||||||
|
// Apply decorations to ALL visible split-editors showing this document, not
|
||||||
|
// just the first match — each editor pane has its own decoration layer.
|
||||||
|
for (const editor of vscode.window.visibleTextEditors) {
|
||||||
|
if (editor.document === document) {
|
||||||
|
editor.setDecorations(this.agentType, agentRanges);
|
||||||
|
editor.setDecorations(this.humanType, humanRanges);
|
||||||
|
}
|
||||||
|
}
|
||||||
this.renderStatus(s);
|
this.renderStatus(s);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+25
-7
@@ -11,19 +11,33 @@ import * as path from "node:path";
|
|||||||
import { emptyArtifact, serializeArtifact, type Artifact } from "./model";
|
import { emptyArtifact, serializeArtifact, type Artifact } from "./model";
|
||||||
|
|
||||||
export class CoauthorStore {
|
export class CoauthorStore {
|
||||||
/** sidecar paths this store just wrote, so the shared watcher can ignore them. */
|
/**
|
||||||
private readonly selfWrites = new Set<string>();
|
* Pending-write counts keyed by absolute sidecar path. A Map (not Set) because
|
||||||
|
* one doc save can trigger TWO store.update() calls (ThreadController then
|
||||||
|
* AttributionController), both writing the same sidecar — a Set would let the
|
||||||
|
* second watcher event leak through as "external" (comment-UI flicker).
|
||||||
|
*/
|
||||||
|
private readonly selfWrites = new Map<string, number>();
|
||||||
|
|
||||||
/** @param rootDir absolute workspace-folder root that owns the `.threads/` tree. */
|
/** @param rootDir absolute workspace-folder root that owns the `.threads/` tree. */
|
||||||
constructor(private readonly rootDir: string) {}
|
constructor(private readonly rootDir: string) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Delete-and-report: true iff `fsPath` was a sidecar we just wrote via
|
* Decrement-and-report: true iff `fsPath` was a sidecar this store just
|
||||||
* `update`. The shared watcher (extension.ts) calls this to suppress
|
* wrote via `update`. Each successful `update` increments the counter once;
|
||||||
* self-write events before fanning out to the controllers.
|
* each call here decrements it (deleting the key at zero). The shared watcher
|
||||||
|
* (extension.ts) calls this to suppress self-write events before fanning out
|
||||||
|
* to the controllers.
|
||||||
*/
|
*/
|
||||||
consumeSelfWrite(fsPath: string): boolean {
|
consumeSelfWrite(fsPath: string): boolean {
|
||||||
return this.selfWrites.delete(fsPath);
|
const count = this.selfWrites.get(fsPath);
|
||||||
|
if (!count) return false;
|
||||||
|
if (count === 1) {
|
||||||
|
this.selfWrites.delete(fsPath);
|
||||||
|
} else {
|
||||||
|
this.selfWrites.set(fsPath, count - 1);
|
||||||
|
}
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** `.threads/<repo-relative-docPath>.json` (spec §6.3). */
|
/** `.threads/<repo-relative-docPath>.json` (spec §6.3). */
|
||||||
@@ -61,8 +75,12 @@ export class CoauthorStore {
|
|||||||
for (const id of Object.keys(artifact.anchors)) {
|
for (const id of Object.keys(artifact.anchors)) {
|
||||||
if (!referenced.has(id)) delete artifact.anchors[id];
|
if (!referenced.has(id)) delete artifact.anchors[id];
|
||||||
}
|
}
|
||||||
this.selfWrites.add(this.sidecarPath(docPath));
|
|
||||||
this.save(docPath, artifact);
|
this.save(docPath, artifact);
|
||||||
|
// Increment AFTER the synchronous write: fs.writeFileSync completes before
|
||||||
|
// this line executes, and watcher events are delivered async, so marking
|
||||||
|
// post-write is race-free for suppression purposes.
|
||||||
|
const p = this.sidecarPath(docPath);
|
||||||
|
this.selfWrites.set(p, (this.selfWrites.get(p) ?? 0) + 1);
|
||||||
return artifact;
|
return artifact;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -88,4 +88,22 @@ describe("CoauthorStore", () => {
|
|||||||
});
|
});
|
||||||
expect(Object.keys(store.load("d.md")!.anchors)).toEqual([]);
|
expect(Object.keys(store.load("d.md")!.anchors)).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("consumeSelfWrite counts multiple writes to the same sidecar (double-save suppression)", () => {
|
||||||
|
const store = new CoauthorStore(root);
|
||||||
|
store.update("d.md", (a) => {
|
||||||
|
a.anchors["a_t"] = { fingerprint: { text: "t", before: "", after: "", lineHint: 0 } };
|
||||||
|
a.threads.push({ id: "t1", anchorId: "a_t", status: "open", messages: [] });
|
||||||
|
});
|
||||||
|
store.update("d.md", (a) => {
|
||||||
|
a.attributions.push({
|
||||||
|
id: "at1", anchorId: "a_t", author: { kind: "human", id: "ben" },
|
||||||
|
createdAt: "2026-06-10T00:00:00.000Z", updatedAt: "2026-06-10T00:00:00.000Z",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
const p = store.sidecarPath("d.md");
|
||||||
|
expect(store.consumeSelfWrite(p)).toBe(true);
|
||||||
|
expect(store.consumeSelfWrite(p)).toBe(true);
|
||||||
|
expect(store.consumeSelfWrite(p)).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user