diff --git a/docs/specs/agent-environments/write-form-calls/spec.md b/docs/specs/agent-environments/write-form-calls/spec.md index 4ebb9ad1e7..8af063d1cf 100644 --- a/docs/specs/agent-environments/write-form-calls/spec.md +++ b/docs/specs/agent-environments/write-form-calls/spec.md @@ -32,11 +32,12 @@ 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; on a match, the type's creates and updates take that order. + 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. - **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 as their recorded answers did. + 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 fda6316b28..eac5b07aee 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -757,10 +757,12 @@ 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 spelled -(`null`, `[]` or `{}`) as its answer spelled them, and stores the fields of both with -the item read-back's clock precision. Updates answer empty lists and maps as -their answers spelled them too. A `reference` is an object holding fields of the +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 +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 entity a sent member names, as the state holds it (a parent beside `parentId`): the member is accepted with any value, serving looks the entity up and refuses a call naming one the environment does not hold or holds in more than one @@ -853,7 +855,8 @@ 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 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; on a match, the type's creates +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 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/environmentStateBulkAnswer.ts b/src/application/environment/environmentStateBulkAnswer.ts index 3f7cf2510c..09288283d2 100644 --- a/src/application/environment/environmentStateBulkAnswer.ts +++ b/src/application/environment/environmentStateBulkAnswer.ts @@ -172,7 +172,15 @@ export const answerBulk = ( const answer = writeAnswer( serving, request, - (recorded) => writtenBulkResponse(plan.response!, recorded, written.value), + (recorded) => + writtenBulkResponse( + plan.response!, + recorded, + written.value.map((entity, index) => ({ + ...entity, + sent: Object.keys(targets.value[index]!.body ?? {}), + })), + ), { source: "state", view: update ? "update" : "delete", diff --git a/src/application/environment/environmentStateFormAnswer.test.ts b/src/application/environment/environmentStateFormAnswer.test.ts index 365104400f..ef1df5d352 100644 --- a/src/application/environment/environmentStateFormAnswer.test.ts +++ b/src/application/environment/environmentStateFormAnswer.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { BULK_RENAMED_LABEL_ROWS, + ECHOED_EMPTY_ROWS, formLabelRows, LISTED_ID_ROWS, NOTE_DEFAULT_ROWS, @@ -43,6 +44,20 @@ describe("served creates and updates modelled on form recordings", () => { ).toMatchObject({ message: expect.stringContaining('"name"') }); }); + it("answers an empty member a call sent as it was sent, whichever spelling the recorded answer used", () => { + const labels = servedFrom(ECHOED_EMPTY_ROWS); + const tags = (sent: JsonValue) => { + const body = labels.answer("post_labels", { + body: { name: `Spare ${JSON.stringify(sent)}`, tags: sent }, + }).body; + if (typeof body !== "object" || body === null || Array.isArray(body)) + throw new Error("Expected an object body"); + return body["tags"]; + }; + expect(tags([])).toEqual([]); + expect(tags(null)).toBeNull(); + }); + it("moves a renamed entity to where the collection's order puts its new name", () => { const labels = servedFrom(RENAMED_LABEL_ROWS); labels.answer("patch_labels_by_id", { id: "1", body: { name: "Zinc" } }); diff --git a/src/application/environment/environmentStateWriteAnswer.ts b/src/application/environment/environmentStateWriteAnswer.ts index 2ce24420d9..656b8598ef 100644 --- a/src/application/environment/environmentStateWriteAnswer.ts +++ b/src/application/environment/environmentStateWriteAnswer.ts @@ -334,7 +334,11 @@ export const writeAnswer = ( serving: StateServing, request: WriteRequest, rendered: - | { readonly fields: JsonObject | undefined; readonly id: string } + | { + readonly fields: JsonObject | undefined; + readonly id: string; + readonly sent?: readonly string[]; + } | (( recorded: JsonValue | undefined, ) => Result), @@ -520,7 +524,7 @@ const answerCreate = ( const answer = writeAnswer( serving, request, - { fields: fields.value, id }, + { fields: fields.value, id, sent: Object.keys(call.body ?? {}) }, state, ); if (!answer.ok) return answer; @@ -592,7 +596,11 @@ const answerUpdate = ( const answer = writeAnswer( serving, request, - { fields: written.value, id: entity.id }, + { + fields: written.value, + id: entity.id, + sent: Object.keys(call.body ?? {}), + }, { source: "state", view: "update", diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index b3502c24e9..0bc1d81d4b 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -711,8 +711,10 @@ export class EnvironmentStateWriteSimulation { }); for (const name of shown.confirmed) { this.#drafts.get(name)!.confirmed.push(record.ordinal); - if (showsPlacement(view, outcome, this.#inserted, name)) + if (showsPlacement(view, outcome, this.#inserted, name)) { this.#placed.add(name); + if (view.shape === "list") this.#narrowInserts(view, record, name); + } } return shown.unseen; } @@ -821,17 +823,12 @@ export class EnvironmentStateWriteSimulation { for (const key of failed.keys) this.#taint.fail(key); } - /** - * The successful check with the collection sorted by a field an update the read exercised writes, when the read - * lists the whole collection in that order and no create of the type was placed by a sort that excludes it; the - * type's creates then insert by it, and its updates move their entity by it. A sort the creates chose by a field the - * update did not write does not stop this, since both may have fit every earlier read. - */ - #retrySort( + /** What `use` makes of a read's items when the read lists the whole collection the state holds, the only read that shows the order it is kept in. */ + #wholeListing( view: Extract, record: StateRecordFacts, - exercised: ReadonlySet, - ): CheckOutcome | undefined { + use: (items: readonly JsonObject[]) => T, + ): T | undefined { const line = this.#line(view, record); const idField = this.#types.get(view.type)?.id_field; if (line === undefined || idField === undefined) return undefined; @@ -840,6 +837,53 @@ export class EnvironmentStateWriteSimulation { .members(view.type, line.collection) .map((entity) => entity.id), ); + return this.#input.io.withBody(record.ordinal, (body) => { + const items = + body === undefined ? undefined : listItems(body, view.pointer); + if ( + items === undefined || + items.length < 2 || + items.length !== held.size || + !items.every((item) => + held.has(scalarText(ownMember(item, idField)) ?? ""), + ) + ) + return undefined; + return use(items); + }); + } + + /** A whole-collection read that placed a create leaves it only the sorted inserts whose order the read follows. */ + #narrowInserts( + view: Extract, + record: StateRecordFacts, + name: string, + ): void { + const draft = this.#drafts.get(name)!; + const kept = this.#wholeListing(view, record, (items) => + draft.candidates.filter( + (candidate) => + candidate.rule !== "sorted" || + same(candidate, draft.plan.insert) || + orderedBy(items, candidate.field, candidate.direction), + ), + ); + if (kept !== undefined) draft.candidates = kept; + } + + /** + * The successful check with the collection sorted by a field an update the read exercised writes, when the read + * lists the whole collection in that order and no create of the type was placed by a sort that excludes it; the + * type's creates then insert by it, and its updates move their entity by it. A sort the creates chose by a field the + * update did not write does not stop this when every whole-collection read that placed them followed this order too. + */ + #retrySort( + view: Extract, + record: StateRecordFacts, + exercised: ReadonlySet, + ): CheckOutcome | undefined { + const line = this.#line(view, record); + if (line === undefined) return undefined; const written = new Set( [...exercised].flatMap((name) => { const { plan } = this.#drafts.get(name)!; @@ -858,27 +902,16 @@ export class EnvironmentStateWriteSimulation { draft.plan.target === view.type && draft.observed !== undefined, ); - const rules = this.#input.io.withBody(record.ordinal, (body) => { - const items = - body === undefined ? undefined : listItems(body, view.pointer); - // Only a read of the whole collection shows the order it is kept in. - if ( - items === undefined || - items.length < 2 || - items.length !== held.size || - !items.every((item) => - held.has(scalarText(ownMember(item, idField)) ?? ""), - ) - ) - return []; - return [...written].flatMap((field) => - (["ascending", "descending"] as const).flatMap((direction) => - orderedBy(items, field, direction) - ? [{ rule: "sorted", field, direction } as const] - : [], + const rules = + this.#wholeListing(view, record, (items) => + [...written].flatMap((field) => + (["ascending", "descending"] as const).flatMap((direction) => + orderedBy(items, field, direction) + ? [{ rule: "sorted", field, direction } as const] + : [], + ), ), - ); - }); + ) ?? []; for (const rule of rules.filter( (candidate) => current === undefined || !same(candidate, current), )) { diff --git a/src/domain/analysisView/environmentStateFormWrites.test.ts b/src/domain/analysisView/environmentStateFormWrites.test.ts index 38819bde61..7a5fba91e8 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 { + CREATION_ORDER_ROWS, DERIVED_TEXT_ROWS, formLabelRows, ID_OR_TITLE_ROWS, @@ -141,6 +142,16 @@ describe("collections kept sorted", () => { }); }); + it("learns no sort from a rename that an earlier read of the creates' order rules out", () => { + const byTitle = { rule: "sorted", field: "title", direction: "ascending" }; + expect(planOf(CREATION_ORDER_ROWS, "post_folders").insert).not.toEqual( + byTitle, + ); + expect(planOf(CREATION_ORDER_ROWS, "put_folders_by_id")).toMatchObject({ + served: false, + }); + }); + 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/environmentStateWriteApply.ts b/src/domain/analysisView/environmentStateWriteApply.ts index 6e0e63c18c..d9f0007d96 100644 --- a/src/domain/analysisView/environmentStateWriteApply.ts +++ b/src/domain/analysisView/environmentStateWriteApply.ts @@ -529,6 +529,8 @@ export const writtenResponse = ( readonly id: string; /** Generated clocks keep the response's spelling independently of the stored entity's. */ readonly clocks?: readonly string[]; + /** Members the call sent, which the answer shows as stored. */ + readonly sent?: readonly string[]; }, optional: readonly string[] = [], ): Applied => { @@ -569,8 +571,12 @@ export const writtenResponse = ( fields.map((field) => { const value = ownMember(written.fields!, field)!; const answered = ownMember(shown, field); - // An answer spells an empty list or map as its recording did, whichever spelling the state holds. - if (answered !== undefined && emptySpellings(answered, value)) + // 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 @@ -590,7 +596,11 @@ export const writtenResponse = ( export const writtenBulkResponse = ( response: WriteResponse, recorded: JsonValue | undefined, - written: readonly { readonly fields: JsonObject; readonly id: string }[], + written: readonly { + readonly fields: JsonObject; + readonly id: string; + readonly sent?: readonly string[]; + }[], ): Applied => { if (response.shape === "empty") return ok(undefined); if (response.shape === "recorded") return ok(recorded); diff --git a/tests/fixtures/environmentStateFormWrites.ts b/tests/fixtures/environmentStateFormWrites.ts index 66c8bebc9e..211789300b 100644 --- a/tests/fixtures/environmentStateFormWrites.ts +++ b/tests/fixtures/environmentStateFormWrites.ts @@ -321,3 +321,48 @@ export const ID_OR_TITLE_ROWS: readonly TimedCaptureRow[] = (() => { ["GET", "/api/folders", [renamed, alpha, bravo, charlie]], ]; })(); + +/** + * Two seeded folders and two creates kept in creation order, which a list read after both shows (`Charlie` after + * `Zulu`), then a rename whose read happens to follow title order. + */ +export const CREATION_ORDER_ROWS: readonly TimedCaptureRow[] = (() => { + const [alpha, bravo] = [folder(1, "Alpha"), folder(2, "Bravo")]; + const [zulu, charlie] = [folder(3, "Zulu"), folder(4, "Charlie")]; + const renamed = folder(3, "Aardvark", 2); + return [ + ["GET", "/api/folders", [alpha, bravo]], + ["GET", "/api/folders/1", alpha], + ["POST", "/api/folders", zulu, 200, { title: "Zulu" }], + ["GET", "/api/folders/3", zulu], + ["GET", "/api/folders", [alpha, bravo, zulu]], + ["POST", "/api/folders", charlie, 200, { title: "Charlie" }], + ["GET", "/api/folders/4", charlie], + ["GET", "/api/folders", [alpha, bravo, zulu, charlie]], + ["PUT", "/api/folders/3", renamed, 200, { title: "Aardvark", version: 1 }], + ["GET", "/api/folders/3", renamed], + ["GET", "/api/folders", [renamed, alpha, bravo, charlie]], + ]; +})(); + +/** Two label creates that send and echo empty tags, one `null` and one `[]`, as an application storing what it is sent does. */ +export const ECHOED_EMPTY_ROWS: readonly TimedCaptureRow[] = (() => { + const tagged = (entity: Entity, tags: JsonValue): Entity => ({ + ...entity, + tags, + }); + const [kitchen, tools] = [tagged(KITCHEN, null), tagged(TOOLS, null)]; + const garden = tagged(label(3, "Garden"), null); + const attic = tagged(label(4, "Attic"), []); + return [ + ["GET", "/api/labels", [kitchen, tools]], + ["GET", "/api/labels/1", kitchen], + ["GET", "/api/labels/2", tools], + ["POST", "/api/labels", garden, 201, formBody(garden)], + ["GET", "/api/labels/3", garden], + ["GET", "/api/labels", [garden, kitchen, tools]], + ["POST", "/api/labels", attic, 201, formBody(attic)], + ["GET", "/api/labels/4", attic], + ["GET", "/api/labels", [attic, garden, kitchen, tools]], + ]; +})();