client_logs.tester: mark our own sessions, not the family's

Telling a real report from one of our test runs was impossible. Three
no_first_frame rows from a development emulator sat in a production log
dump and were only spotted because an emulator decoder name happened to
be visible in a field logged for something unrelated.

Third time this silent drop has cost us — hidden profiles.pinHash, then
client_logs.flavor, now this. A field the client sends does not exist
until a migration declares it, and PocketBase says nothing. Caught only
because the added field was read back off a measured session.

NOT deployed. The reader (amber-app/tools/reports.py) works without this
column by recognising an emulator in `device` and a test address from the
resolved account, so nothing is blocked on the redeploy.

docs/telemetry-contract.md

@@ -21,11 +21,33 @@ older than 14 days nightly.
 | `error`   | `uncaught`          | `{library}` — Flutter framework errors           |
 | `player`  | `exo_error`         | `{host, anime}` — native player error            |
 | `player`  | `av_delay_applied`  | `{audioMs, host}` — user dialed in an audio offset (the "a track falls behind" signal) |
-| `session` | `session_summary`   | `{host, durationS, watchedS, stalls, audioDelayMs, anime}` on player close |
+| `session` | `session_summary`   | one row per playback — see below. **Written only by `SessionFeedbackService._write`.** |
+| `qa`      | `qa_session`        | `{sid, lines[], truncated?}` — the QA decision log for one playback (issue #92), off by default |
 
 `host` is only the stream's `scheme://host` — **never** a full signed URL or an
 addon token (redacted client-side in `TelemetryService.redactUrl` before write).
 
+### `session_summary`, and the two numbers that are not the same
+
+`{sessionId, title, probeKey, host, durationS, watchedS, positionS, stalls,
+droppedFrames, renderedFrames, maxConsecutiveDropped, problemAtS[], promptOutcome,
+rating?}` plus the player's provenance fields (`forcedAudio`, `sideloadedSubs`,
+`resumed`, `seeks`, `seekStormMax`, codec/height/bitrate).
+
+- **`watchedS` is elapsed watch-clock time. `positionS` is where playback got to.**
+  They differ whenever someone seeks, rewatches, or stares at a spinner — a session
+  that never started reads `watchedS: 48, positionS: 0`.
+- **`sessionId` is on every row**, and is what joins a summary to its `qa_session`
+  lines and to the amber-api health record.
+
+**Before 2026-08-07 this was written twice per Android playback**, by the native
+player *and* by the feedback service, and the two disagreed: the extra row had no
+`sessionId`, `title` or `probeKey`, and put the *position* in `watchedS`. In one
+14-day window that was 542 rows of which only 294 were real. **Any aggregate over
+this collection computed before that date counts Android sessions twice**, and
+rows older than the fix still carry the duplicate. `sessionId:isset = true` is the
+filter that excludes them.
+
 ## How Claude queries it
 Superuser token (same as releases publishing), then filter/sort the collection:
 

pb_migrations/1789500000_client_logs_tester.js

@@ -0,0 +1,51 @@
+/// <reference path="../pb_data/types.d.ts" />
+
+// Add `tester` to client_logs: was this session OURS, or a real viewer's?
+//
+// WHY THIS MIGRATION EXISTS: telling our own test runs from the family's sessions
+// was impossible. Three `no_first_frame` rows from a development emulator sat in
+// the middle of a production log dump for an evening, and the only reason they
+// were not read as a family member's television was that an emulator decoder name
+// (`c2.goldfish.hevc.decoder`) happened to be visible in a field logged for an
+// entirely unrelated reason. That is luck, not a method, and a wrong attribution
+// here sends someone chasing a bug that nobody has.
+//
+// THIS IS THE THIRD TIME the same silent drop has cost us: hidden `profiles.pinHash`
+// PATCHes, then `client_logs.flavor` (see the migration beside this one), now this.
+// PocketBase drops an unknown key in a create payload with no error, no warning and
+// HTTP 200. **A field the client sends does not exist until a migration declares
+// it.** It was caught this time only because the added field was read back off a
+// measured session instead of being assumed to have arrived — `tester: None` on a
+// row whose build definitely sends it.
+//
+// Not folded into `meta`: this is a dimension of every record regardless of event
+// type, exactly like `platform`, `flavor` and `device` beside it, and the query it
+// exists for is "hide our own rows", which should not have to reach into a blob.
+//
+// The reader (`amber-app/tools/reports.py`) deliberately does NOT depend on this
+// column — it also recognises an emulator from `device` and a test address from the
+// resolved account, so it stays correct before this deploys and if a future client
+// omits the field. Belt and braces on purpose, because the column is exactly the
+// kind of thing that silently goes missing.
+
+migrate(
+  (app) => {
+    const logs = app.findCollectionByNameOrId("client_logs")
+    logs.fields.add(
+      new BoolField({
+        name: "tester",
+        // Not required. A bool field in PocketBase defaults to false, which is the
+        // correct reading for every historical row: they came from the family's
+        // devices. Rejecting a record for a missing dimension would trade a whole
+        // diagnostic away for a label.
+        required: false,
+      }),
+    )
+    app.save(logs)
+  },
+  (app) => {
+    const logs = app.findCollectionByNameOrId("client_logs")
+    logs.fields.removeByName("tester")
+    app.save(logs)
+  },
+)