perf(artifacts): serve the library listing from UserArtifactsIndex

backend/scripts/backfill_artifact_user_index_keys.py

@@ -43,9 +43,12 @@
   left alone.
 * **Never resurrects a deleted row.** ``attribute_exists(SK)`` on every
   update, matching the writer's own write-back rule.
-* **Reports what it cannot fix** rather than guessing — a HEAD row with
-  no ``updated_at`` is counted and named, not stamped with a fabricated
-  timestamp that would sort wrongly forever.
+* **Encodes a missing ``updated_at`` rather than inventing one.** Such a
+  row is stamped with an empty timestamp segment
+  (``ARTIFACT##{aid}``), which sorts below every real timestamp and so
+  reads last — where the previous in-memory sort put it. Leaving it
+  unstamped would drop the artifact from a sparse index, and from its
+  owner's library, silently.
 
 Run against dev first, then prod::
 
@@ -113,16 +116,27 @@ def plan_row(item: Dict[str, Any]) -> Dict[str, Any] | None:
         return {"skip": f"unexpected PK {pk!r}"}
     if not artifact_id:
         return {"skip": "no artifact_id attribute"}
-    if not updated_at:
-        # Deliberately not falling back to created_at or "now": GSI2SK is
-        # the sort key the library orders by, and a fabricated timestamp
-        # would order this artifact wrongly for the rest of its life.
-        # Better to name it and let a human decide.
-        return {"skip": "no updated_at attribute"}
-
+    # A row with no `updated_at` is stamped with an EMPTY timestamp
+    # segment, not a fabricated one. Two things make that the right
+    # answer rather than a fudge:
+    #
+    #   * Leaving it unstamped would drop the artifact out of a sparse
+    #     index — and so out of its owner's library — permanently and
+    #     silently. Dropping somebody's oldest artifacts is worse than
+    #     showing them undated.
+    #   * "ARTIFACT##{aid}" sorts BELOW every real timestamp ("#" < any
+    #     digit), so read descending it lands last — exactly where the
+    #     old in-memory sort put undated rows. It encodes "no timestamp"
+    #     honestly instead of inventing one that would sort wrongly
+    #     forever.
+    #
+    # Neither dev nor prod had such a row when this was written; this is
+    # the defensive branch, and it preserves a contract the library's
+    # tests already assert.
     return {
         "gsi2pk": pk,  # GSI2PK is exactly the base PK — USER#{user_id}
         "gsi2sk": f"ARTIFACT#{updated_at}#{artifact_id}",
+        "undated": not updated_at,
     }
 
 
@@ -145,7 +159,11 @@ def backfill(table: Any, apply: bool) -> Dict[str, int]:
             continue
 
         logger.info(
-            "stamp %s %s -> GSI2SK=%s", item.get("PK"), sk, plan["gsi2sk"]
+            "stamp %s %s -> GSI2SK=%s%s",
+            item.get("PK"),
+            sk,
+            plan["gsi2sk"],
+            "  (no updated_at — sorts last)" if plan.get("undated") else "",
         )
         if not apply:
             stats["stamped"] += 1

backend/src/apis/app_api/artifacts/service.py

@@ -398,6 +398,9 @@ def get_render_token_service() -> RenderTokenService:
 # Frozen contract — the HEAD row + SessionIndex keys the artifact writer
 # (backend/src/agents/builtin_tools/artifacts/service.py) emits.
 _SESSION_INDEX = "SessionIndex"
+# Sparse index over HEAD rows only — read the block comment in
+# `list_for_user` before assuming a missing artifact is a query bug.
+_USER_INDEX = "UserArtifactsIndex"
 
 
 class ArtifactListService:
@@ -532,75 +535,82 @@ def heads_for_session(
     def list_for_user(self, *, user_id: str) -> list[dict]:
         """Every artifact the user owns, at HEAD, newest-first.
 
-        One base-table Query, no index. The table is already partitioned
-        by user (`PK=USER#{uid}`), so ownership is enforced by the key
-        rather than re-checked per row the way `list_for_session` has to
-        be — a user-wide list is the query this schema was already
-        shaped for.
-
-        Deliberately not using a GSI. `SessionIndex` is partitioned by
-        session, not user, so it cannot serve this at all; the sparse
-        user index the writer stamps keys for (`GSI2PK`/`GSI2SK`) does
-        not exist yet, and is not needed while the heaviest partition
-        sits far under a 1MB page. See the writer's module docstring.
-
-        Two consequences of reading the base table, both deliberate:
-
-        * The Query spans version rows as well as HEAD rows, so it reads
-          roughly 3x what it returns. Filtering happens here rather than
-          in a FilterExpression because the obvious server-side
-          discriminator (`attribute_exists(GSI1PK)`) would couple "is
-          HEAD" to "is session-indexed" — two facts that only happen to
-          coincide today — and a FilterExpression saves payload, not
-          read capacity, so it buys nothing worth that coupling.
-        * The base table sorts by artifact id (a random uuid4), not by
-          time, so recency ordering is applied here in memory. This is
-          the part that would move server-side behind the user index.
+        Served from `UserArtifactsIndex` (GSI2PK=USER#{uid},
+        GSI2SK=ARTIFACT#{updated_at}#{aid}) with
+        `ScanIndexForward=False`.
+
+        This was a base-table Query on the same partition until the
+        index existed. That worked, but read badly: HEAD and version
+        rows share the partition, so it spanned roughly 3x the rows it
+        returned and then date-sorted them in memory. Only HEAD rows
+        carry the GSI2 keys, so the index holds one row per artifact
+        already in newest-first order — the amplification and the sort
+        both go away, and the ordering comes from the store instead of
+        being recomputed per request.
+
+        ############################################################
+        # This index is SPARSE. A HEAD row without GSI2PK is not stale
+        # in it, it is ABSENT from it — and silently, surfacing as a
+        # library that lists fewer artifacts than the user made.
+        #
+        # Two things keep it complete, and both must stay true:
+        #   * the writer stamps GSI2PK/GSI2SK on BOTH of its write
+        #     paths, and
+        #   * rows predating that (2026-09-04) were stamped by
+        #     `scripts/backfill_artifact_user_index_keys.py`.
+        #
+        # If an environment is ever found listing fewer artifacts than
+        # its table holds, re-run that script before looking anywhere
+        # else. It is idempotent.
+        ############################################################
+
+        Still returns the whole library in one response, paging the
+        index internally. Exposing pagination is a bigger change than it
+        looks: search and the type filter are applied in the SPA today,
+        and a filter that sees only the loaded page is worse than no
+        filter because it looks authoritative — both would have to move
+        server-side in the same change. The index makes that possible
+        whenever it is wanted; it is not wanted yet.
         """
         table = _table()
-        items: list[dict] = []
+        rows: list[dict] = []
         kwargs: dict = {
-            "KeyConditionExpression": Key("PK").eq(f"USER#{user_id}")
-            & Key("SK").begins_with("ARTIFACT#"),
+            "IndexName": _USER_INDEX,
+            "KeyConditionExpression": Key("GSI2PK").eq(f"USER#{user_id}"),
+            # GSI2SK leads with updated_at, so descending IS newest-first.
+            "ScanIndexForward": False,
         }
         try:
             while True:
                 resp = table.query(**kwargs)
-                items.extend(resp.get("Items", []))
+                for item in resp.get("Items", []):
+                    if not item.get("artifact_id"):
+                        continue
+                    rows.append(
+                        {
+                            "artifact_id": item.get("artifact_id", ""),
+                            "version": int(item.get("version", 0)),
+                            "title": item.get("title", ""),
+                            "content_type": item.get(
+                                "content_type", "text/html; charset=utf-8"
+                            ),
+                            # Rows written before these attributes existed
+                            # degrade to an empty string rather than
+                            # dropping out of the library.
+                            "created_at": item.get("created_at") or "",
+                            "updated_at": item.get("updated_at") or "",
+                            "session_id": item.get("session_id") or "",
+                        }
+                    )
                 last = resp.get("LastEvaluatedKey")
                 if not last:
                     break
                 kwargs["ExclusiveStartKey"] = last
         except ClientError as exc:
             raise ArtifactQueryError("artifact library query failed") from exc
 
-        heads = [
-            item for item in items
-            if str(item.get("SK", "")).endswith("#HEAD")
-        ]
-        rows = [
-            {
-                "artifact_id": item.get("artifact_id", ""),
-                "version": int(item.get("version", 0)),
-                "title": item.get("title", ""),
-                "content_type": item.get(
-                    "content_type", "text/html; charset=utf-8"
-                ),
-                # Rows written before these attributes existed degrade to
-                # an empty string rather than dropping out of the library.
-                "created_at": item.get("created_at") or "",
-                "updated_at": item.get("updated_at") or "",
-                "session_id": item.get("session_id") or "",
-            }
-            for item in heads
-            if item.get("artifact_id")
-        ]
-        # Newest-first. Undated legacy rows sort last rather than first,
-        # which an empty-string key would otherwise do.
-        rows.sort(
-            key=lambda row: (bool(row["updated_at"]), row["updated_at"]),
-            reverse=True,
-        )
+        # No sort here on purpose — the index supplied the order. Adding
+        # one back would silently mask a broken sort key.
         return rows
 
     @staticmethod

backend/tests/apis/app_api/artifacts/test_artifact_library.py

@@ -53,8 +53,24 @@ def client(monkeypatch: pytest.MonkeyPatch):
             AttributeDefinitions=[
                 {"AttributeName": "PK", "AttributeType": "S"},
                 {"AttributeName": "SK", "AttributeType": "S"},
+                {"AttributeName": "GSI2PK", "AttributeType": "S"},
+                {"AttributeName": "GSI2SK", "AttributeType": "S"},
             ],
             BillingMode="PAY_PER_REQUEST",
+            # The library endpoint reads this index, so the fixture must
+            # have it. moto raises ResourceNotFoundException without it —
+            # correct, and worth keeping: it is what makes these tests
+            # exercise the index instead of a base-table read.
+            GlobalSecondaryIndexes=[
+                {
+                    "IndexName": "UserArtifactsIndex",
+                    "KeySchema": [
+                        {"AttributeName": "GSI2PK", "KeyType": "HASH"},
+                        {"AttributeName": "GSI2SK", "KeyType": "RANGE"},
+                    ],
+                    "Projection": {"ProjectionType": "ALL"},
+                }
+            ],
         )
         monkeypatch.setenv("DYNAMODB_ARTIFACTS_TABLE_NAME", TABLE)
 
@@ -116,8 +132,12 @@ def _put_artifact(
     if updated_at is not None:
         head["updated_at"] = updated_at
         head["GSI1SK"] = f"ARTIFACT#{updated_at}#{artifact}"
-        head["GSI2PK"] = f"USER#{user_id}"
-        head["GSI2SK"] = f"ARTIFACT#{updated_at}#{artifact}"
+    # GSI2 keys are stamped either way — the post-backfill state of the
+    # table. An undated row carries an empty timestamp segment, which
+    # sorts below every real one, so it reads last instead of dropping
+    # out of the sparse index entirely.
+    head["GSI2PK"] = f"USER#{user_id}"
+    head["GSI2SK"] = f"ARTIFACT#{updated_at or ''}#{artifact}"
     table.put_item(Item=head)
 
 
@@ -242,7 +262,13 @@ def test_library_route_is_not_shadowed_by_the_artifact_id_route(client) -> None:
 def test_paginates_a_partition_larger_than_one_page(client) -> None:
     """The Query loop must drain `LastEvaluatedKey`. Asserted with a
     stub rather than 1MB of fixture rows, since moto pages on real byte
-    size and a realistic partition is far under the limit."""
+    size and a realistic partition is far under the limit.
+
+    Page 1 carries the NEWER row, because that is what the index does:
+    GSI2SK leads with `updated_at` and is read descending. So the
+    service must preserve page order rather than re-sort — a
+    client-side sort would pass here either way and hide a broken sort
+    key."""
     calls: list[dict] = []
 
     class Paged:
@@ -253,14 +279,14 @@ def query(self, **kwargs):
                     "Items": [
                         {
                             "PK": f"USER#{USER_ID}",
-                            "SK": "ARTIFACT#a1#HEAD",
-                            "artifact_id": "a1",
+                            "SK": "ARTIFACT#a2#HEAD",
+                            "artifact_id": "a2",
                             "version": 1,
-                            "title": "One",
+                            "title": "Two",
                             "content_type": "text/markdown",
-                            "created_at": "2026-05-01T09:00:00+00:00",
-                            "updated_at": "2026-05-01T09:00:00+00:00",
-                            "session_id": "s1",
+                            "created_at": "2026-05-02T09:00:00+00:00",
+                            "updated_at": "2026-05-02T09:00:00+00:00",
+                            "session_id": "s2",
                         }
                     ],
                     "LastEvaluatedKey": {"PK": "x", "SK": "y"},
@@ -269,14 +295,14 @@ def query(self, **kwargs):
                 "Items": [
                     {
                         "PK": f"USER#{USER_ID}",
-                        "SK": "ARTIFACT#a2#HEAD",
-                        "artifact_id": "a2",
+                        "SK": "ARTIFACT#a1#HEAD",
+                        "artifact_id": "a1",
                         "version": 1,
-                        "title": "Two",
+                        "title": "One",
                         "content_type": "text/markdown",
-                        "created_at": "2026-05-02T09:00:00+00:00",
-                        "updated_at": "2026-05-02T09:00:00+00:00",
-                        "session_id": "s2",
+                        "created_at": "2026-05-01T09:00:00+00:00",
+                        "updated_at": "2026-05-01T09:00:00+00:00",
+                        "session_id": "s1",
                     }
                 ]
             }

backend/tests/test_backfill_artifact_user_index_keys.py

@@ -165,18 +165,24 @@ def test_spans_every_user(table):
 # ------------------------------------------------------------------
 
 
-def test_refuses_to_fabricate_a_sort_key(table):
-    """A HEAD row with no `updated_at` is reported, never guessed at.
-
-    GSI2SK is what the library orders by; inventing a timestamp would
-    sort that artifact wrongly for the rest of its life, invisibly."""
+def test_encodes_a_missing_timestamp_instead_of_inventing_one(table):
+    """A HEAD row with no `updated_at` is still indexed, with an EMPTY
+    timestamp segment.
+
+    Leaving it unstamped would drop the artifact out of a sparse index —
+    and out of its owner's library — silently. An empty segment sorts
+    below every real timestamp, so descending it reads last, exactly
+    where the previous in-memory sort put undated rows."""
     put_head(table, updated_at=None)
 
     stats = backfill_mod.backfill(table, apply=True)
 
-    assert stats["skipped"] == 1
-    assert stats["stamped"] == 0
-    assert "GSI2PK" not in row(table)
+    assert stats["stamped"] == 1
+    assert stats["skipped"] == 0
+    item = row(table)
+    assert item["GSI2SK"] == "ARTIFACT##a1"
+    # "#" is below every digit, so this sorts under any real timestamp.
+    assert item["GSI2SK"] < "ARTIFACT#2026-01-01T00:00:00+00:00#a1"
 
 
 def test_dry_run_writes_nothing(table):