workspace: don't cache exports from a cycle-interrupted evaluation

In an A<->B cycle, exportsFor(B) run while A is mid-resolution would
compute a wrong export set (assignments dropped by the nested
"circular reference" error) and cache it anyway. Later queries then
served that stale, cycle-broken result even after the cycle-causing
outer call had unwound, giving inconsistent error messages across
repeated evaluations of the same cyclic setup.

Skip caching whenever any line in the result carries a "circular
reference" error - that's a precise, local signal the pass was
cycle-affected, so the sheet gets re-evaluated fresh until an edit
actually breaks the cycle. Normal (non-cyclic) caching is untouched.

Also adds a positive-path test confirming LineResult.error stays
undefined on an ordinary successful line, and a workspace-level test
proving a genuine cycle now reports a stable error across repeated
queries.
This commit is contained in:
Horizontal42 2026-07-02 05:50:40 +03:00
parent 26a6bc283b
commit 0345d2a33f
3 changed files with 24 additions and 1 deletions

View file

@ -396,6 +396,10 @@ describe("cross-sheet references", () => {
expect(r[0].value).toBeNull(); expect(r[0].value).toBeNull();
expect(r[0].error).toBe('no variable "nope" in "Budget"'); expect(r[0].error).toBe('no variable "nope" in "Budget"');
}); });
it("ordinary successful line leaves LineResult.error undefined", () => {
const r = eng.evaluateDocument("$100", resolver);
expect(r[0].error).toBeUndefined();
});
it("propagates a resolver-reported circular reference", () => { it("propagates a resolver-reported circular reference", () => {
const cyc = (): XRefResolution => ({ ok: false, reason: "circular reference" }); const cyc = (): XRefResolution => ({ ok: false, reason: "circular reference" });
const r = eng.evaluateDocument("@A.total", cyc); const r = eng.evaluateDocument("@A.total", cyc);

View file

@ -82,6 +82,20 @@ describe("Workspace cycle detection", () => {
expect(r[1].value).toBeNull(); expect(r[1].value).toBeNull();
expect(r[1].error).toBe("circular reference"); expect(r[1].error).toBe("circular reference");
}); });
it("a two-sheet cycle reports a stable error across repeated queries (no stale cache poisoning)", () => {
const { ws } = makeWorkspace({
a: { title: "A", text: "z = @B.total" },
b: { title: "B", text: "z = @A.total" },
});
const first = ws.evaluateSheet("dash", "@A.z");
const second = ws.evaluateSheet("dash", "@A.z");
expect(first[0].value).toBeNull();
expect(second[0].value).toBeNull();
expect(first[0].error).toBeDefined();
expect(second[0].error).toBeDefined();
expect(second[0].error).toBe(first[0].error);
});
}); });
describe("Workspace.renameSheet", () => { describe("Workspace.renameSheet", () => {

View file

@ -138,7 +138,12 @@ export class Workspace {
} }
} }
const exports: SheetExports = { vars, total: this.engine.totalValueOf(results), last }; const exports: SheetExports = { vars, total: this.engine.totalValueOf(results), last };
this.cache.set(sheet.id, exports); // A cycle-interrupted pass produces incomplete/wrong exports (dropped
// assignments, missing total) — don't let it poison the cache. Re-evaluate
// fresh on every query until the cycle is actually broken by an edit.
if (!results.some((r) => r.error === "circular reference")) {
this.cache.set(sheet.id, exports);
}
return exports; return exports;
} }
} }