From 6976a156b131d0d03a8e3c81542a7edfe5ecce02 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 26 Oct 2026 20:45:08 -0700 Subject: [PATCH] fix(environment): try a learned sort by moving each pending write in turn, as serving does #retrySort sorted the whole collection, which a list could match while serving, moving each renamed entity in turn against the others' values before their own writes, gave another order. The state now records the values each write replaced, and the retry replays the writes since a list in collection order last showed their entities. Co-Authored-By: Claude Opus 5.5 --- docs/web-network-captures.md | 4 +- .../EnvironmentStateWriteSimulation.ts | 25 +++++- .../environmentStateFormWrites.test.ts | 7 ++ .../environmentStateWriteChecks.ts | 82 +++++++++++++++---- tests/fixtures/environmentStateFormWrites.ts | 21 +++++ 5 files changed, 121 insertions(+), 18 deletions(-) diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 9d9488262c..78366a71b6 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -858,7 +858,9 @@ update that writes the field its type's creates are sorted by moves its entity to where the new value sorts (its own `insert`); a bulk update moves each entity it writes in turn, as replay applies them. When a read listing the whole collection in its own order, not one its query sorts, fails after an update and the type's creates are not sorted by a field -that update writes, the view tries the collection sorted by such a field, when +that update writes, the view tries the order serving gives when each write since +a list in the collection's own order showed its entity moves it in turn by such a +field, against the others' values before their own writes, when the entities no write has changed since a list in the collection's own order last showed them already follow it, as serving moves only the entities a write changes, and unless diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index a79229f3f8..1f564d777c 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -42,6 +42,7 @@ import { showsPlacement, SimulatedState, type CheckOutcome, + type PendingWrite, } from "./environmentStateWriteChecks.js"; import { checkedLines, @@ -171,6 +172,8 @@ export class EnvironmentStateWriteSimulation { readonly #inserted = new Map(); /** Targets a write changed or placed since a passing list read last showed them, whose position no read has confirmed. */ readonly #moved = new Set(); + /** The writes to those targets' entities, in order. */ + #pending: PendingWrite[] = []; /** Creates whose insert rule a passing list check relied on, so it is no longer changed. */ readonly #placed = new Set(); /** The order a read showed a type's collections sorted in after an update, when no create of it showed one. */ @@ -428,6 +431,7 @@ export class EnvironmentStateWriteSimulation { const { address } = draft.plan; const bulk = address !== undefined && address.by !== "action"; const apply = (body?: ParsedRequestBody): void => { + this.#state.track(); const context = this.#context( tool.name, body === undefined ? undefined : { ordinal: record.ordinal, body }, @@ -482,6 +486,12 @@ export class EnvironmentStateWriteSimulation { return; } this.#effects.supersede(this.#touched, write.tool, result); + if (result.entity !== undefined) { + const { type, collection, id } = result.entity; + const target = entityTarget(type, collection, id); + const before = this.#state.priorFields(target); + if (before !== undefined) this.#pending.push({ target, before }); + } for (const target of result.targets) { this.#moved.add(target); touchTarget(this.#touched, this.#touchedSince, target, { @@ -710,8 +720,11 @@ export class EnvironmentStateWriteSimulation { outcome.pass && view.shape === "list" && outcome.positional === undefined - ) - for (const target of outcome.targets) this.#moved.delete(target); + ) { + const shown = new Set(outcome.targets); + for (const target of shown) this.#moved.delete(target); + this.#pending = this.#pending.filter((write) => !shown.has(write.target)); + } if (!outcome.pass) { const interfered = new Set([...exercised].filter(interferes)); if (interfered.size === 0) @@ -988,7 +1001,13 @@ export class EnvironmentStateWriteSimulation { ) ) continue; - const before = this.#state.arrange(view.type, line.collection, rule); + // Serving moves each written entity in turn, against the others' values before their own writes. + const before = this.#state.replay( + view.type, + line.collection, + rule, + this.#pending, + ); const outcome = this.#check(view, record); if (outcome?.pass === true) { for (const draft of creates) draft.plan.insert = rule; diff --git a/src/domain/analysisView/environmentStateFormWrites.test.ts b/src/domain/analysisView/environmentStateFormWrites.test.ts index fbe51951ab..acfc193264 100644 --- a/src/domain/analysisView/environmentStateFormWrites.test.ts +++ b/src/domain/analysisView/environmentStateFormWrites.test.ts @@ -18,6 +18,7 @@ import { SHELF_TIE_ROWS, SHELF_GUESS_ROWS, ID_SORTED_READ_ROWS, + UNSORTED_PENDING_ROWS, TITLE_ORDER_ROWS, UNLOADED_ANSWER_ROWS, UNSORTED_SEED_ROWS, @@ -194,6 +195,12 @@ describe("collections kept sorted", () => { expect(planOf(SHELF_GUESS_ROWS, "put_books_by_id").served).toBe(false); }); + it("learns no sort from renames whose moves, made in turn as serving makes them, do not reach the list", () => { + const plan = planOf(UNSORTED_PENDING_ROWS, "put_folders_by_id"); + expect(plan.served).toBe(false); + expect(plan.insert).toBeNull(); + }); + it("learns the sort from a rename after a read its query sorts by id showed an earlier rename", () => { expect(planOf(ID_SORTED_READ_ROWS, "put_books_by_id")).toMatchObject({ served: true, diff --git a/src/domain/analysisView/environmentStateWriteChecks.ts b/src/domain/analysisView/environmentStateWriteChecks.ts index b046022a75..e9420bd8cf 100644 --- a/src/domain/analysisView/environmentStateWriteChecks.ts +++ b/src/domain/analysisView/environmentStateWriteChecks.ts @@ -46,6 +46,12 @@ interface RemovedTarget { } /** The gate's copy of the state: entities in collection order and documents, changed by modelled writes. */ +/** A write since a list in collection order last showed its entity, with the values it replaced (`null` for a create). */ +export interface PendingWrite { + readonly target: string; + readonly before: JsonObject | null; +} + export class SimulatedState { readonly #entities = new Map(); readonly #collections = new Map(); @@ -56,6 +62,8 @@ export class SimulatedState { readonly #named = new Map>(); /** The order a type's collections keep their entities in, as its creates insert them. */ #sortOf: (type: string) => InsertRule | undefined = () => undefined; + /** The values each entity put since `track` replaced, `null` for one a put added. */ + readonly #prior = new Map(); constructor( entities: readonly StateEntity[], @@ -111,6 +119,7 @@ export class SimulatedState { put(entity: StateEntity, index?: number): void { const key = entityTarget(entity.type, entity.collection, entity.id); const held = this.#entities.get(key); + if (!this.#prior.has(key)) this.#prior.set(key, held?.fields ?? null); this.#entities.set(key, entity); if (held !== undefined) { this.#resort(held, entity); @@ -129,22 +138,67 @@ export class SimulatedState { this.#served.remember(entity.type, entity.collection); } - /** Put a collection's entities in the order an insert rule would have placed them, returning the order they had. */ - arrange(type: string, collection: string, rule: InsertRule): string[] { - const target = collectionTarget(type, collection); - const before = this.#collections.get(target) ?? []; - const keys: string[] = []; - for (const key of before) - keys.splice( - insertIndex( - rule, - keys.map((member) => this.#entities.get(member)!.fields), - this.#entities.get(key)!.fields, - ), - 0, + /** Start recording the values the next puts replace. */ + track(): void { + this.#prior.clear(); + } + + /** The values an entity held before its first put since `track`: `null` when that put added it, undefined when none did. */ + priorFields(key: string): JsonObject | null | undefined { + return this.#prior.get(key); + } + + /** + * Put a collection's entities in the order serving gives when each of `writes` moves its entity in turn by `rule`, + * against the others' values before their own writes, or a create inserts it; returns the order they had. + */ + replay( + type: string, + collection: string, + rule: Extract, + writes: readonly PendingWrite[], + ): string[] { + const before = + this.#collections.get(collectionTarget(type, collection)) ?? []; + const held = new Set(before); + const ours = writes.filter((write) => held.has(write.target)); + const first = new Map(); + for (const write of ours) + if (!first.has(write.target)) first.set(write.target, write.before); + const order = before + .filter((key) => first.get(key) !== null) + .map((key) => ({ key, + fields: first.get(key) ?? this.#entities.get(key)!.fields, + })); + for (const [index, { target: key, before: prior }] of ours.entries()) { + const fields = + ours.slice(index + 1).find((later) => later.target === key)?.before ?? + this.#entities.get(key)!.fields; + const at = order.findIndex((member) => member.key === key); + if ((prior === null) !== at < 0) continue; + if (at >= 0) order.splice(at, 1); + const moved = + prior === null || + !same(ownMember(prior, rule.field), ownMember(fields, rule.field)); + order.splice( + moved + ? insertIndex( + rule, + order.map((member) => member.fields), + fields, + ) + : at, + 0, + { key, fields }, + ); + } + if (order.length === before.length) + this.restore( + type, + collection, + order.map((member) => member.key), ); - this.restore(type, collection, keys); return before; } diff --git a/tests/fixtures/environmentStateFormWrites.ts b/tests/fixtures/environmentStateFormWrites.ts index ffd3fe1b3a..d698c8e46f 100644 --- a/tests/fixtures/environmentStateFormWrites.ts +++ b/tests/fixtures/environmentStateFormWrites.ts @@ -550,6 +550,27 @@ export const ID_SORTED_READ_ROWS: readonly TimedCaptureRow[] = (() => { ]; })(); +/** Folders listed out of title order, then two renames before a list sorted by title that moving each in turn cannot reach. */ +export const UNSORTED_PENDING_ROWS: readonly TimedCaptureRow[] = (() => { + const [echo, foxtrot, bravo, delta] = [ + folder(1, "Echo"), + folder(2, "Foxtrot"), + folder(3, "Bravo"), + folder(4, "Delta"), + ]; + const charlie = folder(1, "Charlie", 2); + const alpha = folder(2, "Alpha", 2); + return [ + ["GET", "/api/folders", [echo, foxtrot, bravo, delta]], + ["GET", "/api/folders/1", echo], + ["PUT", "/api/folders/1", charlie, 200, { title: "Charlie", version: 1 }], + ["GET", "/api/folders/1", charlie], + ["PUT", "/api/folders/2", alpha, 200, { title: "Alpha", version: 1 }], + ["GET", "/api/folders/2", alpha], + ["GET", "/api/folders", [alpha, bravo, charlie, delta]], + ]; +})(); + /** A user create that sends `active: false` and is answered so, whose read shows the account active. */ export const ACTIVE_ON_READ_ROWS: readonly TimedCaptureRow[] = (() => { const user = (id: number, name: string, active: boolean): Entity => ({