fix(ask): migrate created/updated to autodate, sort ties by id

ask_threads/ask_messages/ask_log shipped with created/updated typed as
plain `date` fields, which PocketBase never populates — every row's
created has been empty since. pb-add-ask-collections.py now emits
autodate fields (onCreate, plus onUpdate for updated) and migrates the
three live collections in place under --apply, re-adding
idx_ask_messages_thread_created if PocketBase drops it with the field.

--backfill (apply-only) just reports affected row counts: PocketBase
can only set an autodate field to *now*, never a historical value, so
there's nothing safe to backfill.

Since old rows keep an empty created either way, the read side sorts
ask_messages by `created,id` instead of `created` alone, so tied
(all-empty) rows land in a stable, deterministic order rather than
whatever order PocketBase happens to return them in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

scripts/pb-add-ask-collections.py

@@ -13,13 +13,32 @@ appending, never delete or replace unrelated schema.
 
 All rules are left null (superuser only) — the Next.js server is the only
 client of these three collections; there is no browser-direct access.
+
+``created``/``updated`` must be PocketBase ``autodate`` fields (they were
+first shipped as plain ``date`` fields, which PocketBase never populates on
+its own — every row's ``created`` has been empty since). ``--apply`` also
+migrates ``ask_threads``, ``ask_messages`` and ``ask_log`` in place: any
+``created``/``updated`` field still typed ``date`` on those three
+collections is replaced with an ``autodate`` field of the same name.
+Existing rows keep an empty ``created`` either way — PocketBase can't
+backfill an autodate field to a historical value — so ``--backfill``
+(only meaningful with ``--apply``) just reports how many rows are affected
+per collection rather than attempting a write that would do nothing.
 """
 import sys
+import urllib.parse
 
 from pb_env import PB, http, superuser_token
 
 DRY_RUN = "--dry-run" in sys.argv or "--apply" not in sys.argv
 APPLY = "--apply" in sys.argv
+BACKFILL = "--backfill" in sys.argv
+
+MIGRATED_COLLECTIONS = ("ask_threads", "ask_messages", "ask_log")
+ASK_MESSAGES_THREAD_CREATED_INDEX = (
+    "CREATE INDEX `idx_ask_messages_thread_created` "
+    "ON `ask_messages` (`thread`, `created`)"
+)
 
 
 def confirmation_host():
@@ -68,11 +87,15 @@ def select(name, values, required=False, max_select=1):
     }
 
 
+def autodate_field(name):
+    options = {"onCreate": True}
+    if name == "updated":
+        options["onUpdate"] = True
+    return field(name, "autodate", **options)
+
+
 def system_fields():
-    return [
+    return [autodate_field("created"), autodate_field("updated")]
-        field("created", "date", onCreate=True),
-        field("updated", "date", onCreate=True, onUpdate=True),
-    ]
 
 
 def add_fields(token, name, wanted):
@@ -112,6 +135,116 @@ def add_indexes(token, name, wanted):
             sys.exit(f"PATCH {name} indexes failed: {code} {response}")
 
 
+def stale_date_field_names(fields):
+    """Names of ``created``/``updated`` fields still typed ``date``. Pure."""
+    return sorted(
+        f["name"] for f in fields
+        if f.get("name") in ("created", "updated") and f.get("type") == "date"
+    )
+
+
+def migration_fields(existing_fields, stale_names):
+    """The full ``fields`` list for the migration PATCH: every field that
+    isn't one of ``stale_names`` kept as-is, plus a fresh ``autodate``
+    field for each stale name. Omitting the old field's ``id`` here is
+    what makes PocketBase drop it — this list represents the *entire*
+    desired field set. Pure — no network calls, easy to unit test.
+    """
+    kept = [f for f in existing_fields if f.get("name") not in stale_names]
+    return kept + [autodate_field(n) for n in sorted(stale_names)]
+
+
+def migrate_system_fields(token, name):
+    """On one of the three Ask collections, replace any ``created``/
+    ``updated`` field still typed ``date`` with an ``autodate`` field of
+    the same name. Idempotent — a second run finds nothing stale. Existing
+    rows keep an empty ``created``/``updated``; see ``--backfill``.
+    """
+    collection = require_collection(token, name)
+    fields = list(collection.get("fields", []))
+    stale = stale_date_field_names(fields)
+    if not stale:
+        print(f"  {name}: created/updated already autodate")
+        return
+
+    print(f"  {name}: migrate {stale} from date -> autodate")
+    replaced = migration_fields(fields, set(stale))
+    if DRY_RUN:
+        print(f"  {name}: would PATCH fields (delete+recreate {stale})")
+        return
+
+    code, response = http(
+        "PATCH", f"/api/collections/{collection['id']}", {"fields": replaced}, token,
+    )
+    if code != 200:
+        # Some PocketBase versions refuse to delete and add a field with
+        # the same name in a single PATCH — fall back to two PATCHes:
+        # delete the stale fields first, then add the autodate ones.
+        print(f"  {name}: combined PATCH failed ({code}: {response}); trying delete then add")
+        kept = [f for f in fields if f.get("name") not in set(stale)]
+        code, response = http(
+            "PATCH", f"/api/collections/{collection['id']}", {"fields": kept}, token,
+        )
+        if code != 200:
+            sys.exit(f"  {name}: delete PATCH failed: {code} {response}")
+        code, current = http("GET", f"/api/collections/{name}", token=token)
+        if code != 200:
+            sys.exit(f"  {name}: re-fetch after delete failed: {code} {current}")
+        current_fields = list(current.get("fields", []))
+        code, response = http(
+            "PATCH", f"/api/collections/{current['id']}",
+            {"fields": current_fields + [autodate_field(n) for n in stale]}, token,
+        )
+        if code != 200:
+            sys.exit(f"  {name}: add PATCH failed: {code} {response}")
+
+    # The thread/created index is defined on the field we just replaced —
+    # re-add it if PocketBase dropped it along with the old field.
+    if name == "ask_messages":
+        code, current = http("GET", f"/api/collections/{name}", token=token)
+        if code != 200:
+            sys.exit(f"  {name}: re-fetch failed: {code} {current}")
+        current_indexes = list(current.get("indexes", []) or [])
+        if ASK_MESSAGES_THREAD_CREATED_INDEX not in current_indexes:
+            print(f"  {name}: re-adding idx_ask_messages_thread_created (dropped with the field)")
+            code, response = http(
+                "PATCH", f"/api/collections/{current['id']}",
+                {"indexes": current_indexes + [ASK_MESSAGES_THREAD_CREATED_INDEX]}, token,
+            )
+            if code != 200:
+                sys.exit(f"  {name}: index PATCH failed: {code} {response}")
+    print(f"  {name}: migrated")
+
+
+def count_empty_created(token, name):
+    query = urllib.parse.quote("created = ''")
+    code, response = http(
+        "GET", f"/api/collections/{name}/records?perPage=1&filter={query}", token=token,
+    )
+    if code != 200:
+        sys.exit(f"  {name}: could not count empty-created rows: {code} {response}")
+    return response.get("totalItems", 0)
+
+
+def backfill_report(token):
+    """``--backfill``: PocketBase only ever sets an ``autodate`` field to
+    *now*, on create (and on update, if ``onUpdate`` is set) — never to an
+    arbitrary historical value, so there is nothing a script can write
+    into an already-empty ``created`` that would be correct. This just
+    counts the affected rows per collection; it never issues a write.
+    """
+    print("\nbackfill (--backfill): reporting only, autodate can't be set to a past value")
+    for name in MIGRATED_COLLECTIONS:
+        total = count_empty_created(token, name)
+        if total == 0:
+            print(f"  {name}: no rows with empty created")
+        else:
+            print(
+                f"  {name}: {total} row(s) have empty created; left as-is. "
+                "The read side orders these by id (see threads.ts)."
+            )
+
+
 def create_collection(token, name, fields, indexes):
     existing = get_collection(token, name)
     if existing is not None:
@@ -142,13 +275,39 @@ def self_test():
     assert f == {"name": "text", "type": "text", "required": False, "max": 100000}
     s = select("kind", ["user", "assistant", "attachment", "image"])
     assert s["values"] == ["user", "assistant", "attachment", "image"]
-    print("self-test: field builders passed")
+
+    assert system_fields() == [
+        {"name": "created", "type": "autodate", "required": False, "onCreate": True},
+        {"name": "updated", "type": "autodate", "required": False, "onCreate": True, "onUpdate": True},
+    ]
+
+    stale_fields = [
+        {"id": "f1", "name": "created", "type": "date", "onCreate": True},
+        {"id": "f2", "name": "updated", "type": "date", "onCreate": True, "onUpdate": True},
+        {"id": "f3", "name": "title", "type": "text"},
+    ]
+    assert stale_date_field_names(stale_fields) == ["created", "updated"]
+    assert stale_date_field_names([{"name": "created", "type": "autodate"}]) == []
+
+    migrated = migration_fields(stale_fields, {"created", "updated"})
+    assert migrated == [
+        {"id": "f3", "name": "title", "type": "text"},
+        {"name": "created", "type": "autodate", "required": False, "onCreate": True},
+        {"name": "updated", "type": "autodate", "required": False, "onCreate": True, "onUpdate": True},
+    ]
+    # No stale "id" leaks through on the replacement fields — that is what
+    # tells PocketBase to drop the old field and create a new one.
+    assert "id" not in migrated[1] and "id" not in migrated[2]
+
+    print("self-test: field builders and migration helpers passed")
     return 0
 
 
 def main():
     if "--self-test" in sys.argv:
         return self_test()
+    if BACKFILL and not APPLY:
+        sys.exit("--backfill requires --apply")
     if APPLY and confirmation_host() != PB:
         sys.exit(
             "REFUSING --apply: pass --confirm-host with the exact configured "
@@ -166,6 +325,13 @@ def main():
 
     print(f"target: {PB}   mode: {'DRY RUN' if DRY_RUN else 'APPLY'}")
 
+    print("\nmigrate created/updated to autodate:")
+    for collection_name in MIGRATED_COLLECTIONS:
+        if get_collection(token, collection_name) is None:
+            print(f"  {collection_name}: does not exist yet, nothing to migrate")
+            continue
+        migrate_system_fields(token, collection_name)
+
     users = require_collection(token, "users")
     sites = require_collection(token, "sites")
     users_id = users["id"]
@@ -248,6 +414,9 @@ def main():
         ],
     )
 
+    if BACKFILL:
+        backfill_report(token)
+
     print("\nDRY RUN. No schema changes written." if DRY_RUN else "\nSchema applied.")
     return 0
 

src/app/api/chat/threads/[id]/route.test.ts

@@ -67,6 +67,13 @@ describe("/api/chat/threads/[id]", () => {
     expect(body.rows).toHaveLength(1);
   });
 
+  it("GET sorts messages by created then id, so equal-created rows are stable", async () => {
+    getOneThreadMock.mockResolvedValue({ id: "t1", owner: "u1" });
+    getFullListMessagesMock.mockResolvedValue([]);
+    await GET(req("GET", { "x-ask-user": "u1" }), { params });
+    expect(getFullListMessagesMock.mock.calls[0][0]).toMatchObject({ sort: "created,id" });
+  });
+
   it("PATCH renames and archives", async () => {
     getOneThreadMock.mockResolvedValue({ id: "t1", owner: "u1" });
     updateThreadMock.mockResolvedValue({ id: "t1", title: "Renamed", archived: true });

src/app/api/chat/threads/[id]/route.ts

@@ -41,9 +41,12 @@ export async function GET(
   const thread = await ownedThread(pb, id, actor.id);
   if (!thread) return NOT_FOUND;
 
+  // `,id` is a stable tiebreaker for rows sharing the same `created` (in
+  // particular pre-autodate-migration rows, which all have an empty
+  // `created`) — see the matching note in lib/chat/threads.ts.
   const rows = await pb.collection("ask_messages").getFullList({
     filter: pb.filter("thread = {:thread}", { thread: id }),
-    sort: "created",
+    sort: "created,id",
   });
 
   return Response.json({ thread, rows });

src/lib/chat/threads.test.ts

@@ -128,7 +128,7 @@ describe("loadThreadEntries", () => {
       { role: "user", text: "hi" },
       { role: "assistant", text: "hello", toolCalls: [] },
     ]);
-    expect(getFullListMock.mock.calls[0][0]).toMatchObject({ sort: "created" });
+    expect(getFullListMock.mock.calls[0][0]).toMatchObject({ sort: "created,id" });
   });
 });
 

src/lib/chat/threads.ts

@@ -124,12 +124,19 @@ export async function getRecentActionIds(threadId: string): Promise<string[]> {
   return last?.action_ids ?? [];
 }
 
-/** Loads a thread's rows from PocketBase, ordered by `created`, and maps them. */
+/**
+ * Loads a thread's rows from PocketBase, ordered by `created`, and maps
+ * them. `,id` is a stable tiebreaker for rows sharing the same `created` —
+ * in particular the pre-autodate-migration rows, which all have an empty
+ * `created` and would otherwise come back in an undefined order. Old test
+ * threads from before the migration may still show slightly out of order
+ * among themselves; there's no recoverable insertion order for those.
+ */
 export async function loadThreadEntries(threadId: string): Promise<ChatEntry[]> {
   const pb = await pbAdmin();
   const rows = (await pb.collection("ask_messages").getFullList({
     filter: pb.filter("thread = {:threadId}", { threadId }),
-    sort: "created",
+    sort: "created,id",
   })) as unknown as ThreadMessageRow[];
   return mapRowsToEntries(rows);
 }