From 3de7d9a7aebe3bff3e708ea70f9a6ed30a737e0d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 26 Oct 2026 15:41:33 -0700 Subject: [PATCH] fix(environment): keep unloaded create answers, check bulk answers with sent members, narrow inserts only on collection-order reads - A create whose item read loaded values its answer had not records them (`observation.read.unloaded`), and a served create's answer keeps them as the recorded answer showed them, a zero-time owner included, while the state holds the read's. - Replay checks a bulk update's recorded answers with each target's sent members, as serving answers them, so echoed `null` and `[]` no longer make the plan body-unusable. - A whole-collection read whose query sorts it shows the query's order, so it no longer narrows a create's sorted insert candidates. Co-Authored-By: Claude Opus 5.5 --- .../write-form-calls/spec.md | 10 ++- docs/web-network-captures.md | 11 ++- .../environmentStateFormAnswer.test.ts | 10 ++- .../environmentStateWriteAnswer.ts | 1 + .../EnvironmentStateWriteSimulation.ts | 15 +++- .../environmentStateFormWrites.test.ts | 16 ++++ .../analysisView/environmentStateView.ts | 6 ++ .../environmentStateWriteApply.ts | 84 +++++++++++++------ .../analysisView/environmentStateWriteBulk.ts | 10 ++- .../environmentStateWriteCreate.ts | 11 ++- .../environmentStateWriteObserve.ts | 17 ++++ .../environmentStateWriteRules.ts | 2 + tests/fixtures/environmentStateFormWrites.ts | 56 +++++++++++++ 13 files changed, 211 insertions(+), 38 deletions(-) diff --git a/docs/specs/agent-environments/write-form-calls/spec.md b/docs/specs/agent-environments/write-form-calls/spec.md index 8af063d1cf..dd0b8012c9 100644 --- a/docs/specs/agent-environments/write-form-calls/spec.md +++ b/docs/specs/agent-environments/write-form-calls/spec.md @@ -32,12 +32,14 @@ natural-language tasks ask for, or withheld them: the new value sorts, in replay and serving (a bulk update moving each entity in turn), and its plan carries that `insert`. When a read listing the whole collection fails after an update and the type's creates are not sorted by a field the update writes, replay tries the collection sorted by such a field, unless a create was placed by a sort - that excludes it or a whole-collection read that placed a create did not follow it; on a match, the type's creates - and updates take that order. + that excludes it or a whole-collection read in collection order (not sorted by its query) that placed a create did + not follow it; on a match, the type's creates and updates take that order. - **Unloaded answers.** A create's later item read is taken as its observation when it differs from the answer only in values the answer had not loaded: empty, zero, `false` or Go's zero time, or objects whose members are each the same - or loaded so. An unsent update field that changes only between `null`, `[]` and `{}` is unchanged. Write answers - spell empty lists and maps the call did not send as their recorded answers did, and a sent one as sent. + or loaded so; the plan points to those values (`observation.read.unloaded`), which a served create's answer keeps as + the recorded answer showed them while the state holds the read's. An unsent update field that changes only between + `null`, `[]` and `{}` is unchanged. Write answers, bulk answers and replay's checks of them included, spell empty + lists and maps the call did not send as their recorded answers did, and a sent one as sent. - **Unruled fields.** A later create that stored a field no rule writes otherwise than the plan keeps it is `unexplained-change` at the create, so its recorded entity is held and later reads check other writes as before. - **Skill.** The two scratch creates differ in what they send, the second only what a person must give, however they diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 5d032c03c3..f546cd2575 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -751,15 +751,17 @@ fields keep the observed value. When the first read of a created entity before the next write shows fields the create's answer lacks, its clocks to another precision, or values the answer had not loaded (an empty, zero or `false` value, Go's zero time, or an object whose members are each loaded so), the observed -entity takes them (`observation.read`). An item read +entity takes them (`observation.read`, whose `unloaded` points to the values the +answer had not loaded). An item read that agrees with the answer otherwise lends its fields and clock spellings and no longer confirms the create; one that disagrees or omits an answer field its item view renders still checks it. Clock agreement uses the coarser recorded precision, including microseconds. A list read only adds fields the answer lacks and still checks it. Serving answers a create in its answer's shape and clock precision, with empty lists and maps the call -did not send spelled (`null`, `[]` or `{}`) as its answer spelled them, and -stores the fields of both with the item read-back's clock precision. Updates +did not send spelled (`null`, `[]` or `{}`) as its answer spelled them and the +values it had not loaded as it answered them, and stores the fields of both, +as the item read-back shows them, with its clock precision. Updates answer empty lists and maps they did not send as their answers spelled them too; a member a call sent is answered as sent. A `reference` is an object holding fields of the @@ -857,7 +859,8 @@ it writes in turn, as replay applies them. When a read listing the whole collection 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, unless a create was placed by a sort that excludes it or a read listing the whole -collection that placed a create did not follow it; on a match, the type's creates +collection in its own order, not one its query sorts, that placed a create did +not follow it; on a match, the type's creates and updates take that order, as Grafana keeps folders by title though no earlier read could tell that from creation order, or from id order where both ascend. diff --git a/src/application/environment/environmentStateFormAnswer.test.ts b/src/application/environment/environmentStateFormAnswer.test.ts index ef1df5d352..1f9024f3a7 100644 --- a/src/application/environment/environmentStateFormAnswer.test.ts +++ b/src/application/environment/environmentStateFormAnswer.test.ts @@ -128,7 +128,15 @@ describe("served creates and updates modelled on form recordings", () => { const tasks = servedFrom(UNLOADED_ANSWER_ROWS); expect( tasks.answer("post_tasks", { body: { title: "Coil the ropes" } }).body, - ).toMatchObject({ title: "Coil the ropes", assignees: [] }); + ).toMatchObject({ + id: 2, + title: "Coil the ropes", + assignees: [], + owner: { id: 1, name: "", created: "0001-01-01T00:00:00Z" }, + }); + expect(tasks.answer("get_tasks_by_id", { id: "2" }).body).toMatchObject({ + owner: { id: 1, name: "Rea Admin" }, + }); expect( tasks.answer("patch_tasks_by_id", { id: "1", body: { done: true } }).body, ).toMatchObject({ done: true, assignees: [] }); diff --git a/src/application/environment/environmentStateWriteAnswer.ts b/src/application/environment/environmentStateWriteAnswer.ts index 656b8598ef..9354e43e47 100644 --- a/src/application/environment/environmentStateWriteAnswer.ts +++ b/src/application/environment/environmentStateWriteAnswer.ts @@ -375,6 +375,7 @@ export const writeAnswer = ( rule.rule === "clock" ? [rule.field] : [], ) : [], + unloaded: plan.observation?.read?.unloaded ?? [], }, optionalFields(plan), ); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index 0bc1d81d4b..242d7f1b1a 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -853,12 +853,25 @@ export class EnvironmentStateWriteSimulation { }); } - /** A whole-collection read that placed a create leaves it only the sorted inserts whose order the read follows. */ + /** A whole-collection read in collection order that placed a create leaves it only the sorted inserts whose order the read follows; one its query sorts shows the query's order instead. */ #narrowInserts( view: Extract, record: StateRecordFacts, name: string, ): void { + const sent = new Set( + [...new URLSearchParams(record.query ?? "").keys()].map((key) => + key.toLowerCase(), + ), + ); + if ( + Object.entries(view.query?.parameters ?? {}).some( + ([parameter, { role }]) => + (role === "sort" || role === "direction") && + sent.has(parameter.toLowerCase()), + ) + ) + return; const draft = this.#drafts.get(name)!; const kept = this.#wholeListing(view, record, (items) => draft.candidates.filter( diff --git a/src/domain/analysisView/environmentStateFormWrites.test.ts b/src/domain/analysisView/environmentStateFormWrites.test.ts index 7a5fba91e8..df19c858a9 100644 --- a/src/domain/analysisView/environmentStateFormWrites.test.ts +++ b/src/domain/analysisView/environmentStateFormWrites.test.ts @@ -5,6 +5,7 @@ import { type TimedCaptureRow, } from "../../../tests/fixtures/analysisView.js"; import { + BULK_EMPTY_TAG_ROWS, CREATION_ORDER_ROWS, DERIVED_TEXT_ROWS, formLabelRows, @@ -13,6 +14,7 @@ import { MINIMAL_CONTACT_ROWS, NOTE_DEFAULT_ROWS, RENAMED_LABEL_ROWS, + SHELF_SORTED_ROWS, TITLE_ORDER_ROWS, UNLOADED_ANSWER_ROWS, } from "../../../tests/fixtures/environmentStateFormWrites.js"; @@ -152,6 +154,20 @@ describe("collections kept sorted", () => { }); }); + it("learns the name order from a rename when a read sorted by its query placed the create", () => { + const byName = { rule: "sorted", field: "name", direction: "ascending" }; + expect(planOf(SHELF_SORTED_ROWS, "put_books_by_id")).toMatchObject({ + served: true, + insert: byName, + }); + }); + + it("serves a bulk update whose recordings echo the empty values each call sent", () => { + expect(planOf(BULK_EMPTY_TAG_ROWS, "patch_labels")).toMatchObject({ + served: true, + }); + }); + it("moves each written entity in turn, against the others' values before their own writes", () => { const entity = (id: string, name: string) => ({ id, fields: { name } }); const [a, b, c, d] = ["a", "b", "c", "d"].map((name, index) => diff --git a/src/domain/analysisView/environmentStateView.ts b/src/domain/analysisView/environmentStateView.ts index 7fe2b2cd03..a3768d4ef2 100644 --- a/src/domain/analysisView/environmentStateView.ts +++ b/src/domain/analysisView/environmentStateView.ts @@ -361,6 +361,12 @@ const writePlanSchema = z.strictObject({ ordinal: ordinalSchema, pointer: z.string(), shape: z.enum(["item", "list"]), + unloaded: z + .array(z.string()) + .exactOptional() + .describe( + "JSON pointers into the entity to values the response had not loaded, which answers keep as the response showed them.", + ), }) .exactOptional() .describe( diff --git a/src/domain/analysisView/environmentStateWriteApply.ts b/src/domain/analysisView/environmentStateWriteApply.ts index d9f0007d96..5e15eac04e 100644 --- a/src/domain/analysisView/environmentStateWriteApply.ts +++ b/src/domain/analysisView/environmentStateWriteApply.ts @@ -531,6 +531,8 @@ export const writtenResponse = ( readonly clocks?: readonly string[]; /** Members the call sent, which the answer shows as stored. */ readonly sent?: readonly string[]; + /** Pointers into the entity to values the recorded answer had not loaded, which the answer keeps as recorded. */ + readonly unloaded?: readonly string[]; }, optional: readonly string[] = [], ): Applied => { @@ -563,33 +565,63 @@ export const writtenResponse = ( ...Object.keys(shown).filter(held), ...optional.filter((field) => !Object.hasOwn(shown, field) && held(field)), ]; - return ok( - replaceAt( - recorded, - pointer, - Object.fromEntries( - fields.map((field) => { - const value = ownMember(written.fields!, field)!; - const answered = ownMember(shown, field); - // An answer spells an empty list or map the call did not send as its recording did. - if ( - answered !== undefined && - written.sent?.includes(field) !== true && - emptySpellings(answered, value) - ) - return [field, answered]; - const clock = - written.clocks?.includes(field) === true - ? readClock(answered) - : undefined; - return [ - field, - clock === undefined ? value : respellClock(value, clock.format), - ]; - }), - ), - ), + const entity = Object.fromEntries( + fields.map((field): [string, JsonValue] => { + const value = ownMember(written.fields!, field)!; + const answered = ownMember(shown, field); + // An answer spells an empty list or map the call did not send as its recording did. + if ( + answered !== undefined && + written.sent?.includes(field) !== true && + emptySpellings(answered, value) + ) + return [field, answered]; + const clock = + written.clocks?.includes(field) === true + ? readClock(answered) + : undefined; + return [ + field, + clock === undefined ? value : respellClock(value, clock.format), + ]; + }), ); + const answer = (written.unloaded ?? []).reduce( + (rendered, path) => { + const tokens = pointerTokens(path); + const kept = memberAt(shown, tokens); + return kept === undefined || written.sent?.includes(tokens[0]!) === true + ? rendered + : withMemberAt(rendered, tokens, kept); + }, + entity, + ); + return ok(replaceAt(recorded, pointer, answer)); +}; + +/** The value under these member names, nested objects included. */ +const memberAt = ( + value: JsonValue | undefined, + tokens: readonly string[], +): JsonValue | undefined => + tokens.reduce( + (inner, token) => (isObject(inner) ? ownMember(inner, token) : undefined), + value, + ); + +/** A copy with the value under these member names replaced, when the object holds every one of them. */ +const withMemberAt = ( + object: JsonObject, + tokens: readonly string[], + value: JsonValue, +): JsonObject => { + const [first, ...rest] = tokens; + if (first === undefined || !Object.hasOwn(object, first)) return object; + if (rest.length === 0) return { ...object, [first]: value }; + const inner = object[first]; + return isObject(inner) + ? { ...object, [first]: withMemberAt(inner, rest, value) } + : object; }; /** The shared bulk renderer: fixed acknowledgements, or one first-template element per written entity. */ diff --git a/src/domain/analysisView/environmentStateWriteBulk.ts b/src/domain/analysisView/environmentStateWriteBulk.ts index 7d5c1786ba..7c1b321931 100644 --- a/src/domain/analysisView/environmentStateWriteBulk.ts +++ b/src/domain/analysisView/environmentStateWriteBulk.ts @@ -229,7 +229,15 @@ export const applyBulk = ( : result?.kind === "applied" ? result.responseFields : undefined; - return fields === undefined ? [] : [{ id: target.id, fields }]; + return fields === undefined + ? [] + : [ + { + id: target.id, + fields, + sent: Object.keys(named[index]!.body ?? {}), + }, + ]; }); const fits = planned.shape === response.shape && diff --git a/src/domain/analysisView/environmentStateWriteCreate.ts b/src/domain/analysisView/environmentStateWriteCreate.ts index 5429a933e0..53be933326 100644 --- a/src/domain/analysisView/environmentStateWriteCreate.ts +++ b/src/domain/analysisView/environmentStateWriteCreate.ts @@ -63,6 +63,7 @@ import { laterObservation, newEntityIn, loadedLater, + unloadedPointers, withRead, type Observed, } from "./environmentStateWriteObserve.js"; @@ -143,10 +144,18 @@ const withReadBack = ( const object = withRead(own.object, later.object, shape); if (same(object, own.object)) return own; if (shape === "item") context.observed(later.ordinal); + // Answers keep the values the response had not loaded, which the state holds as the item read shows them. + const unloaded = + shape === "item" ? unloadedPointers(own.object, later.object) : []; return { ...own, object, - read: { ordinal: later.ordinal, pointer: later.pointer, shape }, + read: { + ordinal: later.ordinal, + pointer: later.pointer, + shape, + ...(unloaded.length === 0 ? {} : { unloaded }), + }, }; }; diff --git a/src/domain/analysisView/environmentStateWriteObserve.ts b/src/domain/analysisView/environmentStateWriteObserve.ts index 9af48afbc0..68d1f58f7e 100644 --- a/src/domain/analysisView/environmentStateWriteObserve.ts +++ b/src/domain/analysisView/environmentStateWriteObserve.ts @@ -24,6 +24,8 @@ export interface ObservedRead { readonly ordinal: number; readonly pointer: string; readonly shape: "item" | "list"; + /** Pointers into the entity to values the response had not loaded and the item read shows loaded. */ + readonly unloaded?: string[]; } /** Whether two clocks agree at the coarser recorded fraction precision, retaining all digits through nanoseconds. */ @@ -73,6 +75,21 @@ export const loadedLater = ( loadedLater(response[key], ownMember(read, key)), )); +/** Pointers to the values a create's response had not loaded where a later read shows others, within nested objects too. */ +export const unloadedPointers = ( + response: JsonObject, + read: JsonObject, + prefix = "", +): string[] => + Object.entries(response).flatMap(([key, value]) => { + const loaded = ownMember(read, key); + const pointer = `${prefix}${pointerTo(key)}`; + if (loaded === undefined || same(value, loaded)) return []; + if (isObject(value) && isObject(loaded) && Object.keys(value).length > 0) + return unloadedPointers(value, loaded, pointer); + return unloaded(value) ? [pointer] : []; + }); + /** A create's response object with a read's fields: an item read's values win, a list read only adds fields the response lacks. */ export const withRead = ( response: JsonObject, diff --git a/src/domain/analysisView/environmentStateWriteRules.ts b/src/domain/analysisView/environmentStateWriteRules.ts index a417e57d86..cad517f8ab 100644 --- a/src/domain/analysisView/environmentStateWriteRules.ts +++ b/src/domain/analysisView/environmentStateWriteRules.ts @@ -298,6 +298,8 @@ export interface StateWritePlan { readonly ordinal: number; readonly pointer: string; readonly shape: "item" | "list"; + /** Pointers into the entity to values the response had not loaded, which answers keep as it showed them. */ + readonly unloaded?: string[]; }; } | null; readonly response: WriteResponse | null; diff --git a/tests/fixtures/environmentStateFormWrites.ts b/tests/fixtures/environmentStateFormWrites.ts index 211789300b..33077fe913 100644 --- a/tests/fixtures/environmentStateFormWrites.ts +++ b/tests/fixtures/environmentStateFormWrites.ts @@ -366,3 +366,59 @@ export const ECHOED_EMPTY_ROWS: readonly TimedCaptureRow[] = (() => { ["GET", "/api/labels", [attic, garden, kitchen, tools]], ]; })(); + +/** Two bulk label renames that send the empty tags each label holds as the form does, `null` on one and `[]` on the other, and echo them. */ +export const BULK_EMPTY_TAG_ROWS: readonly TimedCaptureRow[] = (() => { + const tagged = (entity: Entity, tags: JsonValue): Entity => ({ + ...entity, + tags, + }); + const [kitchen, tools] = [tagged(KITCHEN, null), tagged(TOOLS, [])]; + const [larder, shed] = [ + { ...kitchen, name: "Larder" }, + { ...tools, name: "Shed" }, + ]; + return [ + ["GET", "/api/labels", [kitchen, tools]], + ["GET", "/api/labels/1", kitchen], + ["GET", "/api/labels/2", tools], + [ + "PATCH", + "/api/labels", + [larder], + 200, + [{ id: 1, name: "Larder", tags: null }], + ], + ["GET", "/api/labels/1", larder], + ["PATCH", "/api/labels", [shed], 200, [{ id: 2, name: "Shed", tags: [] }]], + ["GET", "/api/labels/2", shed], + ["GET", "/api/labels", [larder, shed]], + ]; +})(); + +/** + * Two seeded books kept in id order, which their names follow too, a create a read sorted by shelf places by its tie + * with a seeded book, then a rename an unsorted read lists in name order. + */ +export const SHELF_SORTED_ROWS: readonly TimedCaptureRow[] = (() => { + const shelved = (id: number, name: string, shelf: string): Entity => ({ + id, + name, + shelf, + }); + const [alpha, bravo] = [shelved(1, "Alpha", "b"), shelved(2, "Bravo", "a")]; + const charlie = shelved(3, "Charlie", "a"); + const renamed = shelved(3, "Aardvark", "a"); + return [ + ["GET", "/api/books", [alpha, bravo]], + ["GET", "/api/books?sort=-id", [bravo, alpha]], + ["GET", "/api/books/1", alpha], + ["GET", "/api/books/2", bravo], + ["POST", "/api/books", charlie, 201, { name: "Charlie", shelf: "a" }], + ["GET", "/api/books/3", charlie], + ["GET", "/api/books?sort=shelf", [bravo, charlie, alpha]], + ["PUT", "/api/books/3", renamed, 200, { name: "Aardvark", shelf: "a" }], + ["GET", "/api/books/3", renamed], + ["GET", "/api/books", [renamed, alpha, bravo]], + ]; +})();