From 1a86362dde895e38cb6af64f81b46f6fb9dc0342 Mon Sep 17 00:00:00 2001 From: Alex Reppel <77722411+REPPL@users.noreply.github.com> Date: Sat, 18 Jul 2026 18:55:04 +0100 Subject: [PATCH] Fix validation upper bound and missing-t0 merge (burst 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Burst 4 of the autonomous correctness hunt — 2 distinct low-severity bugs, both boundary conditions. - validate seeded idx.end from the zero value 0 and grew it only via end > idx.end, while idx.start was correctly seeded on the first entry. For a fully-negative timeline (an external recording predating manifest t0), idx.end stayed 0, so a finding anchored after the real (negative) session end was accepted. idx.end is now seeded on the first entry, symmetric with idx.start. This is the sibling of burst 3's lower-bound fix — the same function's other half. - merge silently produced a nonsense timeline (events ~55 years after the speech) when a hand-edited/exchanged manifest omitted t0_epoch_ms, because BuildEntries anchored interactions to t0=0. Merge now rejects a manifest missing t0_epoch_ms when interactions are present; transcript-only sessions (already session-relative) are unaffected. Both fixes make the code match its documented contract. Docs (merge surface, CLI reference) and DECISIONS.md updated in step. Gates green. Assisted-by: Claude:claude-fable-5 --- .../development/brief/04-surfaces/03-merge.md | 5 ++++ .abcd/work/DECISIONS.md | 12 +++++++++ docs/reference/cli.md | 2 +- internal/analyze/analyze_test.go | 20 +++++++++++++++ internal/analyze/validate.go | 7 +++++- internal/timeline/timeline.go | 10 ++++++++ internal/timeline/timeline_test.go | 25 +++++++++++++++++++ 7 files changed, 79 insertions(+), 2 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/03-merge.md b/.abcd/development/brief/04-surfaces/03-merge.md index c83af16..ad250ba 100644 --- a/.abcd/development/brief/04-surfaces/03-merge.md +++ b/.abcd/development/brief/04-surfaces/03-merge.md @@ -23,6 +23,11 @@ alongside the manifest. session-relative); interactions enter as `src: "event"` entries with epoch milliseconds converted via `t0_epoch_ms`, and are assigned sequential `ev-NNN` IDs at merge time. +- When interactions are present, `t0_epoch_ms` is required: without it the + epoch-millisecond interaction times cannot be placed on the session clock, so + merge rejects the session rather than emit a silently corrupt timeline. A + transcript-only session (no interactions) is already session-relative and is + unaffected. - Entries are stably sorted by time and written one JSON object per line; schema in [`../05-internals/02-schemas.md`](../05-internals/02-schemas.md). - Prints the count: `merged N utterances + M events → /timeline.jsonl`. diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 7792d47..3aae7dd 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -174,3 +174,15 @@ Architecture-shaping decisions graduate to an ADR under newline-less prefix) can no longer leave a partial JSONL line that corrupts one physical record and breaks merge's reader; corrected the false comment claiming `os.File.Write` gives newline atomicity. +- 2026-07-18 — `analyze.indexTimeline` seeds `idx.end` on the first entry (`i == 0 + || end > idx.end`), matching how `idx.start` is seeded, so a fully-negative + timeline (an external recording predating manifest t0) reports its true latest + (still-negative) entry end as `sessionEnd` instead of flooring it at the zero + value 0. Fixes an over-permissive finding-time upper bound that admitted a + finding anchored after the real session end. +- 2026-07-18 — `timeline.Merge` rejects a session that has interactions but a + manifest lacking `t0_epoch_ms` (T0EpochMS == 0), since epoch-millisecond + interaction times cannot be placed on the session clock without it; previously + it used the zero-value anchor and wrote a silently corrupt timeline (~55-year + offsets, nonsense report duration) with exit 0. Transcript-only sessions are + unaffected. diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 5494a64..07e977e 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -72,7 +72,7 @@ testimony merge -session DIR |---|---|---| | `-session` | *(required)* | session directory | -Behaviour: reads `manifest.json` (required), `transcript.jsonl`, and `interactions.jsonl`; converts interaction epoch-millisecond times to session-relative seconds via `t0_epoch_ms`; writes the time-sorted `timeline.jsonl`; prints `merged N utterances + M events → `. A missing `transcript.jsonl` or `interactions.jsonl` counts as zero records rather than an error, so a default audio-only `record` session (which never writes `interactions.jsonl`) still merges to a speech-only timeline. +Behaviour: reads `manifest.json` (required), `transcript.jsonl`, and `interactions.jsonl`; converts interaction epoch-millisecond times to session-relative seconds via `t0_epoch_ms`; writes the time-sorted `timeline.jsonl`; prints `merged N utterances + M events → `. A missing `transcript.jsonl` or `interactions.jsonl` counts as zero records rather than an error, so a default audio-only `record` session (which never writes `interactions.jsonl`) still merges to a speech-only timeline. When interactions are present, `t0_epoch_ms` is required: without it their epoch-millisecond times cannot be placed on the session clock, so merge fails rather than write a corrupt timeline. ## `testimony report` diff --git a/internal/analyze/analyze_test.go b/internal/analyze/analyze_test.go index 064adab..eec774a 100644 --- a/internal/analyze/analyze_test.go +++ b/internal/analyze/analyze_test.go @@ -185,6 +185,26 @@ func TestIngestAcceptsNegativeAnchoredFinding(t *testing.T) { } } +// TestIngestRejectsFindingAfterNegativeSessionEnd is the loose-upper-bound +// regression: when every timeline entry sits at negative session-relative time +// (an external recording predating manifest t0), the session end is the latest +// still-negative entry, not 0. Pre-fix indexTimeline seeded idx.end from the zero +// value 0 and only grew it on `end > idx.end`, so a fully-negative timeline +// reported its end as 0 and admitted a finding anchored after the real session +// end. negativeTimeline's latest end is utt-004's t1 = -1, so t = -0.5 is after +// the session ended and must be rejected. +func TestIngestRejectsFindingAfterNegativeSessionEnd(t *testing.T) { + dir := writeSession(t, negativeTimeline) + answer := `{"rubric":"testimony-analysis/v1","findings":[ + {"id":"F-001","t":-0.5,"type":"bug","severity":3,"quote":"I clicked save and nothing happened", + "evidence":["utt-004","ev-003"],"status":"unverified"} + ]}` + _, err := Ingest(dir, strings.NewReader(answer)) + if err == nil || !strings.Contains(err.Error(), "outside the session") { + t.Fatalf("expected an out-of-range refusal for t after the negative session end, got %v", err) + } +} + // TestIngestRefusesEmptyFindings is the data-loss regression: an empty findings // array must not truncate a prior good findings.jsonl. Pre-fix Ingest wrote an // empty slice with O_TRUNC and reported success. diff --git a/internal/analyze/validate.go b/internal/analyze/validate.go index 7ef0270..1b855fe 100644 --- a/internal/analyze/validate.go +++ b/internal/analyze/validate.go @@ -53,7 +53,12 @@ func indexTimeline(entries []timeline.Entry) timelineIndex { idx.routes[r] = true } } - if end > idx.end { + // Seed idx.end on the first entry, exactly as idx.start is seeded below, so + // the upper bound reflects the true maximum end even when every entry sits + // at negative session-relative time (a recording predating t0, audio-only). + // Growing from the zero value 0 would floor the end at 0 and admit a finding + // anchored after the real (negative) session end. + if i == 0 || end > idx.end { idx.end = end } // Track the earliest entry start so the finding-time lower bound matches diff --git a/internal/timeline/timeline.go b/internal/timeline/timeline.go index 2ac43f1..c2e4335 100644 --- a/internal/timeline/timeline.go +++ b/internal/timeline/timeline.go @@ -156,6 +156,16 @@ func Merge(dir string) (speech, events int, err error) { return 0, 0, fmt.Errorf("read interactions: %w", err) } + // Interaction times are epoch milliseconds; without t0 they cannot be placed + // on the session clock. A manifest lacking t0_epoch_ms leaves T0EpochMS at the + // zero value, which would turn each epoch-ms timestamp into a ~55-year offset + // and write a silently corrupt timeline. Reject it rather than emit nonsense. + // A transcript-only session carries no interactions and is already + // session-relative, so it is unaffected. + if len(ints) > 0 && man.T0EpochMS == 0 { + return 0, 0, fmt.Errorf("manifest is missing t0_epoch_ms; cannot place interactions on the session clock") + } + entries := BuildEntries(man.T0EpochMS, utts, ints) if err := session.WriteJSONL(filepath.Join(dir, session.TimelineFile), entries); err != nil { return 0, 0, fmt.Errorf("write timeline: %w", err) diff --git a/internal/timeline/timeline_test.go b/internal/timeline/timeline_test.go index 80e2d65..5c7e80f 100644 --- a/internal/timeline/timeline_test.go +++ b/internal/timeline/timeline_test.go @@ -3,6 +3,7 @@ package timeline import ( "os" "path/filepath" + "strings" "testing" "github.com/REPPL/Testimony/internal/session" @@ -74,6 +75,30 @@ func TestMergeAudioOnlySession(t *testing.T) { } } +// TestMergeRejectsMissingT0WithInteractions is the missing-anchor regression: a +// session with interactions.jsonl but a manifest lacking t0_epoch_ms cannot place +// those epoch-ms interaction times on the session clock. Pre-fix Merge used the +// zero-value t0 (0), turning each epoch-ms timestamp into a ~55-year offset and +// writing a silently corrupt timeline while exiting 0. Merge must reject it. +func TestMergeRejectsMissingT0WithInteractions(t *testing.T) { + dir := t.TempDir() + // Manifest without t0_epoch_ms (left at the zero value). + if err := session.SaveManifest(dir, session.Manifest{Session: "s"}); err != nil { + t.Fatalf("SaveManifest: %v", err) + } + ints := []Interaction{{T: t0 + 9_500, Kind: "click", Selector: "[data-testid=save-btn]"}} + if err := session.WriteJSONL(filepath.Join(dir, session.InteractionsFile), ints); err != nil { + t.Fatalf("write interactions: %v", err) + } + if _, _, err := Merge(dir); err == nil || !strings.Contains(err.Error(), "t0_epoch_ms") { + t.Fatalf("expected a missing-t0 error, got %v", err) + } + // The corrupt timeline must not have been written. + if _, statErr := os.Stat(filepath.Join(dir, session.TimelineFile)); statErr == nil { + t.Fatalf("timeline.jsonl was written despite the missing t0") + } +} + func TestEventsNearWindow(t *testing.T) { entries := sample() var speech []Entry