Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .abcd/development/brief/04-surfaces/03-merge.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 → <dir>/timeline.jsonl`.
7 changes: 6 additions & 1 deletion .abcd/development/brief/04-surfaces/06-analyze.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ page ([`../05-internals/02-schemas.md`](../05-internals/02-schemas.md)).
`findings.jsonl`. Ingest never trusts the model — nothing lands `confirmed`.
- To protect the retained precision record, ingest refuses to overwrite a
`findings.jsonl` that already holds verdict records; a fresh or verdict-free
file is written cleanly.
file is written cleanly. The guard counts any `kind:"verdict"` line, including
one whose value is outside the closed enum (a hand-edited or shared file), so a
foreign-valued human decision is never silently truncated by a re-ingest.
- An answer with no findings (a bare `[]`, `{"findings":[]}`, or a truncated
file) is refused rather than written: the write truncates, so an empty answer
would otherwise erase a prior good `findings.jsonl` and report success.

## Deferred

Expand Down
2 changes: 1 addition & 1 deletion .abcd/development/brief/05-internals/02-schemas.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ validation boundary; every field below is checked, and `status` is forced to
| Field | Type | Required | Notes |
|---|---|---|---|
| `id` | string | yes | `^F-\d{3}$` (F-NNN, zero-padded); unique within the file |
| `t` | float64 | yes | finding time, session-relative seconds; `0 ≤ t ≤ sessionEnd` |
| `t` | float64 | yes | finding time, session-relative seconds; `sessionStart ≤ t ≤ sessionEnd`, where the bounds are the earliest and latest entry times in `timeline.jsonl` and `sessionStart` is `0` unless the timeline holds negative-time utterances (a recording predating `t0`) |
| `type` | string | yes | one of `bug \| friction \| inconsistency \| preference \| idea` |
| `severity` | int | yes | Nielsen-style `1..4` (cosmetic, minor, major, blocker) |
| `mode` | string | no | `A \| B`, default `A`; only Mode A is produced in this slice |
Expand Down
31 changes: 31 additions & 0 deletions .abcd/work/DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,34 @@ Architecture-shaping decisions graduate to an ADR under
(`bytes.TrimSpace`), matching `analyze.Load`, so an exchanged/hand-edited
session's blank line is skipped as documented rather than crashing merge/report
with "unexpected end of JSON input".
- 2026-07-18 — `analyze.validate` derives the finding-`t` lower bound from the
timeline (earliest entry time, floored at 0) instead of hard-coding 0, so a
finding faithfully anchored to a legitimately negative-time utterance — an
external recording whose `creation_time` predates `t0`, giving a negative
`deriveOffset` — no longer fails the whole transactional ingest.
- 2026-07-18 — `analyze.Ingest` refuses an answer with no findings (bare `[]`,
`{"findings":[]}`, or a truncated file) rather than writing an empty slice with
O_TRUNC, which previously erased a prior good `findings.jsonl` and reported
success.
- 2026-07-18 — `analyze.holdsVerdicts` scans `findings.jsonl` for any raw
`kind:"verdict"` line instead of consulting the enum-filtered `analyze.Load`
slice, so the overwrite guard still fires for a hand-edited/shared file whose
only verdict carries an out-of-enum value, protecting the retained precision
record from a truncating re-ingest.
- 2026-07-18 — `demo.appendRecords` truncates a stream file back to its
pre-write length when a write fails, so a short write (ENOSPC persists a
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.
4 changes: 2 additions & 2 deletions docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 → <path>`. 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 → <path>`. 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`

Expand Down Expand Up @@ -135,7 +135,7 @@ testimony analyze -session DIR -ingest FILE # validate the answer → find

Emit behaviour: writes a single self-contained prompt — the rubric version header (`testimony-analysis/v1`), the second-coder stance, two-pass instructions (segment coding, then session synthesis), the rubric body (five `type` definitions, the `1..4` severity scale, the evidence hard-constraints), the session context (app, participant, tasks), the timeline lines inline, and the required output shape with a worked example. Nothing in the session directory is mutated. The timeline is emitted whole (v1 does not chunk by task boundary; the manifest carries no task timestamps). With `-out FILE` the prompt goes to a file and the command prints `wrote <path>`; otherwise it prints to stdout.

Ingest behaviour: reads the answer from `FILE` (or stdin when `-`), accepting a top-level object with a `findings` array (optionally a `rubric`, which must be a known version) or a bare array. Ingest is the sole validation boundary and never trusts the model. Each finding is decoded with unknown fields disallowed, then checked against every schema rule (see [session directory reference](session-directory.md#findingsjsonl)): id format and uniqueness, `t` within the session, the `type` and `severity` enums, non-empty `evidence` with every id real and at least one spoken `utt-*` anchor, a `quote` that is a verbatim substring of one *cited* evidence utterance, and any `ui` selector/route matching a real event. Validation is transactional — all errors are reported at once and nothing is written on any failure. On success every finding is forced to `status: unverified`, `findings.jsonl` is written, and the command prints `validated N findings → <path> (all unverified)`. Ingest refuses to overwrite a `findings.jsonl` that already holds verdict records.
Ingest behaviour: reads the answer from `FILE` (or stdin when `-`), accepting a top-level object with a `findings` array (optionally a `rubric`, which must be a known version) or a bare array. Ingest is the sole validation boundary and never trusts the model. Each finding is decoded with unknown fields disallowed, then checked against every schema rule (see [session directory reference](session-directory.md#findingsjsonl)): id format and uniqueness, `t` within the session, the `type` and `severity` enums, non-empty `evidence` with every id real and at least one spoken `utt-*` anchor, a `quote` that is a verbatim substring of one *cited* evidence utterance, and any `ui` selector/route matching a real event. Validation is transactional — all errors are reported at once and nothing is written on any failure. On success every finding is forced to `status: unverified`, `findings.jsonl` is written, and the command prints `validated N findings → <path> (all unverified)`. An answer with no findings (a bare `[]`, `{"findings":[]}`, or a truncated file) is refused rather than written, so it cannot erase a prior `findings.jsonl`. Ingest refuses to overwrite a `findings.jsonl` that already holds verdict records — counting any `kind:"verdict"` line, even one whose value is outside the closed enum.

## `testimony review`

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/session-directory.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Ingest validates every finding against the merged timeline and is the sole valid
| Field | Type | Required | Meaning |
|---|---|---|---|
| `id` | string | yes | `F-NNN`, zero-padded (`^F-\d{3}$`); unique within the file |
| `t` | number | yes | finding time, session-relative seconds; within `[0, sessionEnd]` |
| `t` | number | yes | finding time, session-relative seconds; within `[sessionStart, sessionEnd]` (the earliest and latest timeline entry times; `sessionStart` is `0` unless the timeline holds negative-time utterances from a recording predating `t0`) |
| `type` | string | yes | one of `bug`, `friction`, `inconsistency`, `preference`, `idea` |
| `severity` | integer | yes | usability-severity scale `1..4`: cosmetic, minor, major, blocker |
| `mode` | string | no | `A` or `B`, default `A`; only Mode A is produced today |
Expand Down
97 changes: 97 additions & 0 deletions internal/analyze/analyze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,103 @@ func TestIngestRefusesOverwriteWithVerdicts(t *testing.T) {
}
}

// negativeTimeline is a merged timeline whose utterance sits at a negative
// session-relative time, as an external recording predating manifest t0
// legitimately produces (deriveOffset returns creation_time − t0 < 0).
const negativeTimeline = `{"t":-3,"src":"speech","id":"utt-004","payload":{"speaker":"P1","t1":-1,"text":"Hm. I clicked save and nothing happened. No message."}}
{"t":-2.5,"src":"event","id":"ev-003","payload":{"kind":"click","route":"#general","selector":"[data-testid=save-btn]","text":"Save"}}
`

// TestIngestAcceptsNegativeAnchoredFinding is the negative-time regression: a
// finding anchored to a legitimately negative-time utterance must ingest. Pre-fix
// validate hard-coded the lower bound at 0, so it rejected `t` outside [0, end]
// and failed the whole (transactional) ingest for such a session.
func TestIngestAcceptsNegativeAnchoredFinding(t *testing.T) {
dir := writeSession(t, negativeTimeline)
answer := `{"rubric":"testimony-analysis/v1","findings":[
{"id":"F-001","t":-3.0,"type":"bug","severity":3,"quote":"I clicked save and nothing happened",
"evidence":["utt-004","ev-003"],"status":"unverified"}
]}`
findings, err := Ingest(dir, strings.NewReader(answer))
if err != nil {
t.Fatalf("Ingest of a negative-anchored finding: %v", err)
}
if len(findings) != 1 || findings[0].T != -3.0 {
t.Fatalf("got %+v, want one finding at t=-3", findings)
}
}

// 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.
func TestIngestRefusesEmptyFindings(t *testing.T) {
dir := writeSession(t, timelineFixture)
if _, err := Ingest(dir, strings.NewReader(goodAnswer)); err != nil {
t.Fatalf("first Ingest: %v", err)
}
path := filepath.Join(dir, session.FindingsFile)
before, _ := os.ReadFile(path)

for _, empty := range []string{`{"findings":[]}`, `[]`} {
if _, err := Ingest(dir, strings.NewReader(empty)); err == nil || !strings.Contains(err.Error(), "no findings") {
t.Fatalf("empty answer %q: expected a no-findings refusal, got %v", empty, err)
}
after, _ := os.ReadFile(path)
if string(before) != string(after) {
t.Fatalf("empty answer %q truncated a prior findings.jsonl", empty)
}
}
}

// TestIngestRefusesOverwriteWithForeignVerdict is the guard-precision
// regression: findings.jsonl whose only verdict line carries an out-of-enum
// value (a hand-edited/shared file) must still block a truncating re-ingest.
// Pre-fix holdsVerdicts consulted analyze.Load, which drops out-of-enum verdicts,
// so the guard saw none and the human-decision record was overwritten.
func TestIngestRefusesOverwriteWithForeignVerdict(t *testing.T) {
dir := writeSession(t, timelineFixture)
if _, err := Ingest(dir, strings.NewReader(goodAnswer)); err != nil {
t.Fatalf("first Ingest: %v", err)
}
path := filepath.Join(dir, session.FindingsFile)
f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY, 0o644)
if err != nil {
t.Fatalf("open: %v", err)
}
// "confirm" is not in the closed enum (confirmed|rejected|duplicate).
f.WriteString(`{"kind":"verdict","finding":"F-001","verdict":"confirm","at":"2026-07-18"}` + "\n")
f.Close()

before, _ := os.ReadFile(path)
if _, err := Ingest(dir, strings.NewReader(goodAnswer)); err == nil || !strings.Contains(err.Error(), "refusing to overwrite") {
t.Fatalf("expected overwrite refusal for a foreign-verdict file, got %v", err)
}
after, _ := os.ReadFile(path)
if string(before) != string(after) {
t.Fatalf("findings.jsonl was modified despite the refusal")
}
}

func TestEffectiveStatusLastWins(t *testing.T) {
findings := []Finding{{ID: "F-001"}, {ID: "F-002"}}
verdicts := []Verdict{
Expand Down
42 changes: 39 additions & 3 deletions internal/analyze/ingest.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package analyze

import (
"bufio"
"bytes"
"encoding/json"
"errors"
Expand Down Expand Up @@ -60,6 +61,13 @@ func Ingest(dir string, r io.Reader) ([]Finding, error) {
if rubric != "" && !knownRubrics[rubric] {
return nil, fmt.Errorf("unknown rubric %q (expected %s)", rubric, RubricVersion)
}
// An empty findings array (a bare `[]`, `{"findings":[]}`, or a truncated
// answer file) is a no-op, not a truncating write: the write below opens with
// O_TRUNC, so proceeding would erase a prior good findings.jsonl and report
// success. Refuse it, mirroring the verdict-overwrite guard.
if len(raws) == 0 {
return nil, fmt.Errorf("answer contains no findings; refusing to overwrite %s", session.FindingsFile)
}

var (
findings []Finding
Expand Down Expand Up @@ -141,14 +149,42 @@ func decodeFinding(raw json.RawMessage) (Finding, error) {
}

// holdsVerdicts reports whether an existing findings.jsonl already contains any
// verdict record. A missing file is not an error (false, nil).
// verdict record. A missing file is not an error (false, nil). It scans for raw
// kind:"verdict" lines rather than reusing analyze.Load, whose verdict slice is
// filtered to the closed enum (confirmed|rejected|duplicate): a hand-edited or
// shared file whose only verdict lines carry a foreign or typo'd value would
// otherwise slip past the guard and have its human-decision records truncated by
// a re-ingest — exactly the precision history the guard exists to protect.
func holdsVerdicts(dir string) (bool, error) {
_, verdicts, err := Load(dir)
path := filepath.Join(dir, session.FindingsFile)
f, err := os.Open(path)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return false, nil
}
return false, err
}
return len(verdicts) > 0, nil
defer f.Close()

sc := bufio.NewScanner(f)
sc.Buffer(make([]byte, 0, 64*1024), 4*1024*1024)
for sc.Scan() {
raw := sc.Bytes()
if len(bytes.TrimSpace(raw)) == 0 {
continue
}
var probe struct {
Kind string `json:"kind"`
}
if err := json.Unmarshal(raw, &probe); err != nil {
return false, fmt.Errorf("%s: %w", path, err)
}
if probe.Kind == "verdict" {
return true, nil
}
}
if err := sc.Err(); err != nil {
return false, fmt.Errorf("%s: %w", path, err)
}
return false, nil
}
30 changes: 26 additions & 4 deletions internal/analyze/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type timelineIndex struct {
uttText map[string]string
selectors map[string]bool
routes map[string]bool
start float64
end float64
}

Expand All @@ -35,7 +36,7 @@ func indexTimeline(entries []timeline.Entry) timelineIndex {
selectors: map[string]bool{},
routes: map[string]bool{},
}
for _, e := range entries {
for i, e := range entries {
idx.ids[e.ID] = true
end := e.T
switch e.Src {
Expand All @@ -52,9 +53,23 @@ 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
// what the timeline can actually hold: an external recording whose
// creation_time predates the manifest t0 yields a negative offset
// (deriveOffset), so transcript.jsonl and the merged timeline legitimately
// carry negative session-relative times. Hard-coding 0 as the floor would
// reject a faithful finding anchored to such an utterance.
if i == 0 || e.T < idx.start {
idx.start = e.T
}
}
return idx
}
Expand All @@ -77,8 +92,15 @@ func validate(findings []Finding, idx timelineIndex) []error {
seen[f.ID] = i + 1
}

if f.T < 0 || f.T > idx.end {
errs = append(errs, fmt.Errorf("%s: t %g is outside the session [0, %g]", label, f.T, idx.end))
// The floor is the earlier of 0 and the earliest entry start: a normal
// session keeps 0 as the lower bound, while a session with negative-time
// utterances (a recording predating t0) admits a finding anchored there.
lo := 0.0
if idx.start < lo {
lo = idx.start
}
if f.T < lo || f.T > idx.end {
errs = append(errs, fmt.Errorf("%s: t %g is outside the session [%g, %g]", label, f.T, lo, idx.end))
}
if !typeSet[f.Type] {
errs = append(errs, fmt.Errorf("%s: type %q not in bug|friction|inconsistency|preference|idea", label, f.Type))
Expand Down
Loading