Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-action-query-objects.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@agent-native/core": patch
---

Preserve nested object parameters when browser clients call GET actions.
23 changes: 23 additions & 0 deletions packages/core/src/client/use-action.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,29 @@ describe("serializeActionQueryParams", () => {
expect(params.has("empty")).toBe(false);
expect(params.has("none")).toBe(false);
});

it("serializes nested object GET params as JSON", () => {
const tableQuery = {
filters: [
{
propertyId: "status",
operator: "equals",
value: "published",
},
],
sorts: [{ propertyId: "date", direction: "desc" }],
};

const query = serializeActionQueryParams({
documentId: "doc-1",
tableQuery,
});

const params = new URLSearchParams(query);
expect(params.get("documentId")).toBe("doc-1");
expect(params.get("tableQuery")).toBe(JSON.stringify(tableQuery));
expect(query).not.toContain("%5Bobject+Object%5D");
});
});

describe("callAction", () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/client/use-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ function appendActionQueryParam(
}
return;
}
if (typeof value === "object") {
// defineAction restores JSON strings when the schema expects an object.
// Preserve nested GET params instead of collapsing them to "[object Object]".
qs.append(key, JSON.stringify(value));
Comment on lines +210 to +213

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Preserve non-plain GET parameter values

This branch JSON-serializes every object, including Date and URLSearchParams. A Date now arrives JSON-quoted instead of in its prior parseable string form, while URLSearchParams serializes to {} and loses its entries; restrict this path to plain records and retain the existing string conversion for other object types.

Additional Info
Found by 1 of 3 incremental review agents; confirmed from the changed serializer behavior.

Fix in Builder

return;
}
qs.append(key, String(value));
}

Expand Down
46 changes: 45 additions & 1 deletion packages/core/src/server/action-routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,23 @@ describe("mountActionRoutes", () => {
baseCurrency: z.string().optional(),
includeSeries: z.boolean().optional(),
limit: z.number().optional(),
tableQuery: z
.object({
filters: z.array(
z.object({
propertyId: z.string(),
operator: z.string(),
value: z.string(),
}),
),
sorts: z.array(
z.object({
propertyId: z.string(),
direction: z.enum(["asc", "desc"]),
}),
),
})
.optional(),
}),
run: async (params: any) => ({ params }),
});
Expand All @@ -836,7 +853,24 @@ describe("mountActionRoutes", () => {
const result = await mounted[0].handler({
_method: "GET",
req: {
url: "http://app.test/_agent-native/actions/instrument-overview?portfolioId=p1&isin=US67066G1040&includeSeries=true&limit=5",
url: `http://app.test/_agent-native/actions/instrument-overview?${new URLSearchParams(
{
portfolioId: "p1",
isin: "US67066G1040",
includeSeries: "true",
limit: "5",
tableQuery: JSON.stringify({
filters: [
{
propertyId: "status",
operator: "equals",
value: "published",
},
],
sorts: [{ propertyId: "date", direction: "desc" }],
}),
},
)}`,
},
});

Expand All @@ -846,6 +880,16 @@ describe("mountActionRoutes", () => {
isin: "US67066G1040",
includeSeries: true,
limit: 5,
tableQuery: {
filters: [
{
propertyId: "status",
operator: "equals",
value: "published",
},
],
sorts: [{ propertyId: "date", direction: "desc" }],
},
},
});
});
Expand Down
873 changes: 873 additions & 0 deletions plans/content-blog-table-performance-research.md

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions templates/content/actions/_batch-utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getDialect, type Dialect } from "@agent-native/core/db";

export function chunks<T>(items: T[], size: number): T[][] {
if (items.length === 0) return [];
const out: T[][] = [];
Expand All @@ -6,3 +8,13 @@ export function chunks<T>(items: T[], size: number): T[][] {
}
return out;
}

export function bulkChunkSizeForColumnCount(
columnCount: number,
dialect: Dialect = getDialect(),
) {
// D1 rejects statements with more than 100 bound params, so derive every
// bulk chunk from the statement's column count instead of a fixed row count.
const budget = dialect === "d1" ? 90 : 900;
return Math.max(1, Math.floor(budget / Math.max(1, columnCount)));
}
2 changes: 2 additions & 0 deletions templates/content/actions/_builder-cms-read-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ describe("Builder CMS read client", () => {
"data.customModelField",
"data.Status",
"data.status",
"data.optionalField",
"data.blocks",
],
fetchImpl: fetchImpl as unknown as typeof fetch,
Expand All @@ -448,6 +449,7 @@ describe("Builder CMS read client", () => {
"data.customModelField": "Preserved",
"data.Status": "Editorial",
"data.status": "published",
"data.optionalField": null,
},
},
],
Expand Down
62 changes: 51 additions & 11 deletions templates/content/actions/_builder-cms-read-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,36 @@ function normalizeBuilderCmsListFieldPath(fieldPath: string) {
return normalized;
}

function preserveProjectedBuilderFieldAbsence(
read: BuilderCmsReadResult,
fieldPaths: readonly string[] | undefined,
rawData: boolean | undefined,
): BuilderCmsReadResult {
if (read.state !== "live" || rawData === true || !fieldPaths?.length) {
return read;
}
const projectedFieldPaths = [
...new Set(
fieldPaths
.map(normalizeBuilderCmsListFieldPath)
.filter((fieldPath): fieldPath is string => fieldPath !== null),
),
];
if (projectedFieldPaths.length === 0) return read;
return {
...read,
entries: read.entries.map((entry) => {
const sourceValues = { ...entry.sourceValues };
for (const fieldPath of projectedFieldPaths) {
if (!Object.prototype.hasOwnProperty.call(sourceValues, fieldPath)) {
sourceValues[fieldPath] = null;
Comment on lines +246 to +248

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Do not mark nested Builder projections as absent

The normalizer accepts paths such as data.author.name, but the source adapter materializes the containing data.author value rather than a data.author.name key. This loop can therefore add data.author.name: null even when the provider returned a real nested value, causing the source-field backfill to skip its refresh and bind an empty property.

Additional Info
Found by 1 of 3 incremental review agents; confirmed against normalizeBuilderCmsApiEntry and builderSourceValuesFromRecord.

Fix in Builder

}
}
return { ...entry, sourceValues };
}),
};
}

export function builderCmsListEntryFields(fieldPaths: readonly string[] = []) {
const fields = new Map<string, string>();
for (const fieldPath of [
Expand Down Expand Up @@ -1273,22 +1303,32 @@ export async function readBuilderCmsContentEntries(args: {
publicKey,
privateKey: privateKey ?? undefined,
});
if (contentApiRead.state === "live") return contentApiRead;
if (contentApiRead.state === "live") {
return preserveProjectedBuilderFieldAbsence(
contentApiRead,
args.fieldPaths,
args.rawData,
);
}
if (!privateKey) return contentApiRead;
}

if (privateKey) {
try {
return await readBuilderCmsContentEntriesViaMcp({
model: args.model,
fieldPaths: args.fieldPaths,
rawData: args.rawData,
limit: args.limit,
maxPages: args.maxPages,
offset: args.offset,
fetchImpl,
privateKey,
});
return preserveProjectedBuilderFieldAbsence(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Do not treat failed MCP hydration as an empty field

The MCP search fallback retains the original entry when per-entry hydration fails. Passing that partial result through absence preservation turns missing projected fields into authoritative nulls, so source-field creation can skip its Builder reread and persist empty values instead of retrying or surfacing the unreadable state.

Additional Info
Found by 1 of 3 incremental review agents; confirmed against the per-entry hydration fallback.

Fix in Builder

await readBuilderCmsContentEntriesViaMcp({
model: args.model,
fieldPaths: args.fieldPaths,
rawData: args.rawData,
limit: args.limit,
maxPages: args.maxPages,
offset: args.offset,
fetchImpl,
privateKey,
}),
args.fieldPaths,
args.rawData,
);
} catch (error) {
return {
state: "error",
Expand Down
52 changes: 31 additions & 21 deletions templates/content/actions/_content-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { accessFilter } from "@agent-native/core/sharing";
import { and, eq, inArray, isNotNull, isNull, or, sql } from "drizzle-orm";

import { schema } from "../server/db/index.js";
import { bulkChunkSizeForColumnCount, chunks } from "./_batch-utils.js";
import {
listContentOrganizationMemberships,
normalizeContentSpaceEmail,
Expand Down Expand Up @@ -32,23 +33,29 @@ export function contentFilesItemId(databaseId: string, documentId: string) {

async function remapItemReferences(db: Db, replacements: Map<string, string>) {
if (!replacements.size) return;
const duplicateIds = [...replacements.keys()];
const remap = (column: any) => {
let expression = sql`CASE ${column}`;
for (const [duplicateId, canonicalId] of replacements)
expression = sql`${expression} WHEN ${duplicateId} THEN ${canonicalId}`;
return sql`${expression} ELSE ${column} END`;
};
for (const table of [
schema.contentSpaceCatalogItems,
schema.contentDatabaseBodyHydrationQueue,
schema.contentDatabaseSourceRows,
schema.contentDatabaseSourceChangeSets,
]) {
await db
.update(table)
.set({ databaseItemId: remap(table.databaseItemId) })
.where(inArray(table.databaseItemId, duplicateIds));
for (const replacementBatch of chunks(
[...replacements.entries()],
bulkChunkSizeForColumnCount(3),
)) {
const batchReplacements = new Map(replacementBatch);
const duplicateIds = [...batchReplacements.keys()];
const remap = (column: any) => {
let expression = sql`CASE ${column}`;
for (const [duplicateId, canonicalId] of batchReplacements)
expression = sql`${expression} WHEN ${duplicateId} THEN ${canonicalId}`;
return sql`${expression} ELSE ${column} END`;
};
for (const table of [
schema.contentSpaceCatalogItems,
schema.contentDatabaseBodyHydrationQueue,
schema.contentDatabaseSourceRows,
schema.contentDatabaseSourceChangeSets,
]) {
await db
.update(table)
.set({ databaseItemId: remap(table.databaseItemId) })
.where(inArray(table.databaseItemId, duplicateIds));
}
}
}

Expand Down Expand Up @@ -170,15 +177,18 @@ async function reconcileDocuments(args: {
}
}
await remapItemReferences(args.db, replacements);
if (deleteIds.size) {
for (const deleteBatch of chunks(
[...deleteIds],
bulkChunkSizeForColumnCount(1),
)) {
await args.db
.delete(schema.contentDatabaseItems)
.where(inArray(schema.contentDatabaseItems.id, [...deleteIds]));
.where(inArray(schema.contentDatabaseItems.id, deleteBatch));
}
if (inserts.length) {
for (const insertBatch of chunks(inserts, bulkChunkSizeForColumnCount(8))) {
await args.db
.insert(schema.contentDatabaseItems)
.values(inserts)
.values(insertBatch)
.onConflictDoNothing();
}
return { inserted: inserts.length, removed: deleteIds.size };
Expand Down
47 changes: 47 additions & 0 deletions templates/content/actions/_database-source-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
serializeSourceMetadataRecord,
sourceSnapshotValuesJsonProjectionSql,
sourceSnapshotDocumentSelection,
sourceSnapshotPageDocumentIds,
sourceValuesForSnapshot,
sourceValuesForSeededSourceRow,
sourceChangeSetKey,
Expand Down Expand Up @@ -103,6 +104,52 @@ function item(id: string, title: string): ContentDatabaseItem {
}

describe("database source helpers", () => {
it("page-scopes primary Builder rows without truncating secondary federation", () => {
expect(
sourceSnapshotPageDocumentIds({
sourceType: "builder-cms",
metadataJson: "{}",
documentIds: ["doc-1", "doc-2"],
}),
).toEqual(["doc-1", "doc-2"]);
expect(
sourceSnapshotPageDocumentIds({
sourceType: "builder-cms",
metadataJson: "{}",
documentIds: [],
}),
).toEqual([]);

expect(
sourceSnapshotPageDocumentIds({
sourceType: "builder-cms",
metadataJson: JSON.stringify({
federation: {
role: "secondary",
keyField: "slug",
normalizationFormula: "lower(trim(value))",
join: {
kind: "identity",
collection: null,
localExpr: "{Slug}",
remoteKeyField: "slug",
normalizationFormula: "lower(trim(value))",
},
},
}),
documentIds: ["doc-1"],
}),
).toBeUndefined();

expect(
sourceSnapshotPageDocumentIds({
sourceType: "local-table",
metadataJson: "{}",
documentIds: ["doc-1"],
}),
).toBeUndefined();
});

it("keeps the provider-bound property when duplicate local labels collide", () => {
const existingFields = [
{
Expand Down
Loading
Loading