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
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
19 changes: 19 additions & 0 deletions .abcd/work/DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,22 @@ 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.
2 changes: 1 addition & 1 deletion docs/reference/cli.md
Original file line number Diff line number Diff line change
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
77 changes: 77 additions & 0 deletions internal/analyze/analyze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,83 @@ 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)
}
}

// 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
}
23 changes: 20 additions & 3 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 @@ -55,6 +56,15 @@ func indexTimeline(entries []timeline.Entry) timelineIndex {
if 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 +87,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
45 changes: 35 additions & 10 deletions internal/demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,45 @@ func (s *server) appendLines(w http.ResponseWriter, r *http.Request, f *os.File,

s.mu.Lock()
defer s.mu.Unlock()
if err := appendRecords(f, lines); err != nil {
// The capture was not persisted; tell the client so it does not treat a
// dropped event as recorded, rather than answering 204 as if it had
// succeeded.
http.Error(w, "capture write failed", http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusNoContent)
}

// appendFile is the subset of *os.File that appendRecords needs; a fake
// satisfies it in tests to exercise the partial-write rollback.
type appendFile interface {
io.Writer
Seek(offset int64, whence int) (int64, error)
Truncate(size int64) error
}

// appendRecords writes each line and its terminating newline to f. os.File.Write
// gives no atomicity guarantee: a full disk fills the remaining space and returns
// a short count, so a bare Write can persist a truncated, newline-less prefix
// (e.g. `{"t":123,"kind":"cl`) before ENOSPC surfaces. That partial line would
// join the next successful write into one malformed physical record and break
// the JSONL reader for the whole file. So on any write error appendRecords
// truncates f back to the length it had before the failing write, so no partial
// line survives — the caller only reports the drop, the file stays clean.
func appendRecords(f appendFile, lines [][]byte) error {
for _, l := range lines {
// Write the record and its terminating newline as a single buffer: a
// partial write (e.g. ENOSPC) can then never land a data line without
// its newline, which would join to the next successful write and produce
// one malformed physical line that later fails merge's JSONL reader.
before, err := f.Seek(0, io.SeekEnd)
if err != nil {
return err
}
if _, err := f.Write(append(l, '\n')); err != nil {
// The capture was not persisted; tell the client so it does not treat
// a dropped event as recorded, rather than answering 204 as if it had
// succeeded.
http.Error(w, "capture write failed", http.StatusInternalServerError)
return
// Best-effort roll back any partial bytes; surface the original error.
f.Truncate(before)
return err
}
}
w.WriteHeader(http.StatusNoContent)
return nil
}

// compactLine canonicalises one accepted JSON value into a single newline-free
Expand Down
53 changes: 53 additions & 0 deletions internal/demo/demo_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package demo

import (
"errors"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -146,6 +147,58 @@ func TestAppendLinesReportsWriteError(t *testing.T) {
}
}

// shortWriteFile is an appendFile whose Write persists a prefix and then errors,
// standing in for a full disk (write(2) fills the remaining space, returns a
// short count, and the next write returns ENOSPC — os.File.Write persists the
// truncated prefix before returning the error).
type shortWriteFile struct {
buf []byte
fail bool // when true, Write keeps only a prefix then returns an error
}

func (f *shortWriteFile) Seek(offset int64, whence int) (int64, error) {
return int64(len(f.buf)), nil
}

func (f *shortWriteFile) Truncate(size int64) error {
f.buf = f.buf[:size]
return nil
}

func (f *shortWriteFile) Write(p []byte) (int, error) {
if f.fail {
half := len(p) / 2
f.buf = append(f.buf, p[:half]...)
return half, errors.New("no space left on device")
}
f.buf = append(f.buf, p...)
return len(p), nil
}

// TestAppendRecordsRollsBackPartialWrite is the ENOSPC regression: a short write
// that persists a newline-less prefix must be truncated away, so the stream file
// never retains a partial line that would corrupt one physical JSONL record and
// break merge's reader for the whole file. Pre-fix appendLines wrote directly
// with no rollback, so the prefix survived.
func TestAppendRecordsRollsBackPartialWrite(t *testing.T) {
f := &shortWriteFile{}
if err := appendRecords(f, [][]byte{[]byte(`{"t":1,"kind":"click"}`)}); err != nil {
t.Fatalf("first append: %v", err)
}
good := string(f.buf)

f.fail = true
if err := appendRecords(f, [][]byte{[]byte(`{"t":2,"kind":"click"}`)}); err == nil {
t.Fatalf("expected a write error on a full disk")
}
if string(f.buf) != good {
t.Fatalf("partial line survived: file is %q, want the clean prefix %q", f.buf, good)
}
if !strings.HasSuffix(string(f.buf), "\n") {
t.Fatalf("file does not end on a newline: %q", f.buf)
}
}

// TestDisplayURL pins the human-facing URL: "localhost" only for the host-less
// default, and the real host otherwise, never the broken "localhost0.0.0.0:8737"
// that concatenating a full -addr after the "localhost" literal produced.
Expand Down