Make timing.client_sent_msg_ts nullable so an absent send time is NULL, not the epoch

[swinney]

Objective

Make timing.client_sent_msg_ts nullable and persist NULL — instead of the Unix epoch — when a chat request supplies no client_sent_msg_ts, so a row never records 1970-01-01T00:00:00Z as if it were a measured send time.

Context you need

What is true today. PR #185 (merged as part of issue #175) made client_sent_msg_ts and client_timeout optional on both chat endpoints. Before it, a request omitting either was refused with HTTP 408 inside _prepare_chat_context and never reached persistence. It now completes, so for the first time such a request writes a timing row.

_parse_chat_request coerces the absent field to 0:

src/interfaces/chat_app/app.py:4680
    client_sent_msg_ts = client_sent_msg_ts / 1000 if client_sent_msg_ts else 0

Both completion paths then convert that 0 and persist it:

• src/interfaces/chat_app/app.py:2550 — streaming (ChatWrapper.stream), written via insert_timing a few lines later, guarded by if message_ids:
• src/interfaces/chat_app/app.py:4767 — non-streaming (get_chat_response), written via self.chat.insert_timing

Both do datetime.fromtimestamp(client_sent_msg_ts, tz=timezone.utc), which for 0 is 1970-01-01T00:00:00+00:00. Line numbers are as of 0365a07de66cf14706af79b4606097087b0b749a (the head of fix/issue-175-optional-client-timeout); re-derive them with grep rather than trusting them, because app.py shifts on nearly every merge.

Why the epoch is there and not NULL. The column forbids it:

src/cli/templates/init.sql:476
    client_sent_msg_ts TIMESTAMPTZ NOT NULL,

insert_timing (src/interfaces/chat_app/app.py:1500) builds a positional tuple and executes SQL_INSERT_TIMING (src/utils/sql.py:70-85), which names all twelve columns. Passing None today raises NotNullViolation; in the non-streaming path that call sits outside the try, so the request would return 500.

Why it was deferred rather than fixed in #185. Shipping the code change without a guaranteed schema migration turns a data-quality defect into a hard 500 on any deployment whose schema predates the migration — and issue #180 records that migrations are not applied to existing deployments. That is this issue's blocker: do not start until #180 is closed, or until you have confirmed by inspection that the migration mechanism reaches existing deployments.

What already exists to build on.

• Migration precedent: src/cli/templates/migrations/add_documents_last_modified.sql and src/cli/templates/migrations/rename_mid_to_message_id.sql. Read both before writing a third — add_documents_last_modified.sql is the one last_modified migration is never applied to existing deployments (ingest then silently no-ops) #180 is about.
• The current behaviour is specified, not accidental: the requirement "A timing row records an absent client send time as a specified sentinel" in openspec/specs/chat-api-request-contract/spec.md (added by fix(chat): treat absent client timeout as no deadline, not expired #185's change fix-issue-175-optional-client-timeout). This issue retires that requirement — expect to write a ## MODIFIED Requirements delta, not a new one bolted alongside.
• Tests that pin the epoch and will need to flip: tests/unit/test_chat_timing_persistence.py, added in 5872eb1b. Three tests; the second (test_the_absent_send_time_is_the_epoch_and_not_a_server_side_substitute) is the one that must go red first.
• Documentation of the sentinel to remove: the client_sent_msg_ts row of the request-body table in docs/docs/api_reference.md, and the [sentinel] link definition.

What does not need changing. No shipped consumer reads this column. src/cli/templates/grafana/archi-default-dashboard.json keys off server_received_msg_ts and msg_duration only — verify with grep -c client_sent_msg_ts on that file (expect 0). msg_duration is computed at app.py:1519 from server_response_msg_ts - server_received_msg_ts and does not involve this column. src/interfaces/chat_app/openai_compat.py:274 synthesises now.timestamp(), so that path never supplies an absent value.

Do not drop the timing row when the send time is absent. Its other ten milestones are real measurements and are what the dashboards plot.

Constraints

• Branch from origin/dev. Open the PR with gh pr create --repo fasrc/archi --base dev. Not upstream/dev (archi-physics/archi) — that fork is ~100 commits behind and is not the working trunk. Never commit to dev directly.
• TDD. Flip the failing test in tests/unit/test_chat_timing_persistence.py first, watch it fail for the right reason, then change the code.
• Gate before every commit: bash scripts/gate.sh, run bare — no pipe, no redirect. It must pass format, lint, tests, and ≥80% diff coverage on changed lines. Never --no-verify.
• No Co-Authored-By or other AI-attribution trailers. Short lowercase commit subjects.
• Use OpenSpec: /opsx:propose → /opsx:apply → /opsx:archive.
• Never merge. A human merges.
• Two changes must land together or the deployment breaks: the migration must be applied before the code that writes NULL runs. State the ordering explicitly in the PR body and in the change's tasks.md. If they cannot be guaranteed to land together, make the writer tolerate both schemas (attempt NULL, fall back to the epoch on NotNullViolation) and say so — a 500 is not an acceptable outcome for a legal request.

Plan

1. Verify the blocker is clear. Confirm last_modified migration is never applied to existing deployments (ingest then silently no-ops) #180 is closed and that a new file in src/cli/templates/migrations/ is actually executed against an existing deployment. If it is not, stop and report — the rest of this plan is unsafe.
2. Schema. Add src/cli/templates/migrations/make_timing_client_sent_nullable.sql (ALTER TABLE timing ALTER COLUMN client_sent_msg_ts DROP NOT NULL;, idempotent, with the same header-comment style as the two existing migrations), and drop NOT NULL from that column in src/cli/templates/init.sql so new deployments match.
3. Tests first. Change tests/unit/test_chat_timing_persistence.py to assert None, watch it fail, and add a case proving a supplied value still round-trips (that discriminator already exists — keep it).
4. Code. At app.py:2550 and app.py:4767, persist None when client_sent_msg_ts is falsey and the converted datetime otherwise. Prefer one small shared helper over two copies of the conditional; both sites are otherwise identical.
5. Spec. MODIFY the sentinel requirement to require NULL, and state that a timing row is still written.
6. Docs. Remove the sentinel sentence and the [sentinel] link definition from docs/docs/api_reference.md; say the column is NULL when the client declares no send time.
7. Separate PR for anything else. If you find yourself repairing api_reference.md line anchors, that is issue #190 — do not fold it in.

Commands

# ground truth for the line numbers above (they drift)
grep -n 'client_sent_msg_ts' src/interfaces/chat_app/app.py
grep -n 'client_sent_msg_ts' src/cli/templates/init.sql src/utils/sql.py

# confirm no dashboard reads the column (expect 0)
grep -c client_sent_msg_ts src/cli/templates/grafana/archi-default-dashboard.json

# red first, then green
python -m pytest tests/unit/test_chat_timing_persistence.py -q

# the gate — bare, no pipe, no redirect
bash scripts/gate.sh

openspec validate <change-name> --strict

Acceptance criteria

• grep -n 'client_sent_msg_ts' src/cli/templates/init.sql shows the column without NOT NULL.
• src/cli/templates/migrations/make_timing_client_sent_nullable.sql exists, is idempotent (running it twice is not an error), and drops the constraint.
• python -m pytest tests/unit/test_chat_timing_persistence.py -q passes with the tests asserting None, and at least one test still proves a supplied timestamp is persisted unchanged.
• grep -rn 'fromtimestamp' src/interfaces/chat_app/app.py shows no site that can be reached with a falsey client_sent_msg_ts.
• grep -c sentinel docs/docs/api_reference.md returns 0.
• bash scripts/gate.sh passes with ≥80% diff coverage on the changed lines.
• openspec validate <change-name> --strict passes, and the change contains a ## MODIFIED Requirements delta retiring the epoch sentinel rather than a second contradictory requirement.
• The PR body states the migration-before-code ordering, or documents the dual-schema fallback and why it was needed.

Start here

Run gh issue view 180 --repo fasrc/archi --json state,title first. If it is still open, do not proceed — comment on this issue saying #180 blocks it and stop. If it is closed, read src/cli/templates/migrations/add_documents_last_modified.sql and whatever now applies it, then start at step 1.

---

Deferred from the round-3 review of PR #185 (fix SHA 5872eb1b), where the epoch was specified as an interim sentinel rather than left accidental. Blocked on #180. Related: #190 (self-verifying api_reference.md anchors).