Skip to content

feat(discovery): discover omp session transcripts in the pi adapter - #129

Open
mdc2122 wants to merge 1 commit into
kunchenguid:mainfrom
mdc2122:fm/backpass-omp-adapter-20
Open

mdc2122 wants to merge 1 commit into
kunchenguid:mainfrom
mdc2122:fm/backpass-omp-adapter-20

Conversation

@mdc2122

@mdc2122 mdc2122 commented Sep 16, 2026

Copy link
Copy Markdown

Summary

omp (Oh My Pi) writes pi-compatible version:3 JSONL session transcripts, but the pi adapter could not see them. Two gaps:

  1. Store root: omp writes to ~/.omp/agent/sessions/ (it honors PI_CODING_AGENT_DIR, defaulting to ~/.omp/agent), which was not among the adapter's roots.
  2. Header layout: omp prepends a fixed-width {"type":"title",...,"pad":"…"} record as line 1 (rewritten in place when the title generates), so the {"type":"session","version":3,"id":…,"cwd":…} entry is line 2. classify() read line 1 only and required type:"session" + cwd, so every omp file classified as null.

Changes

  • storeSpecs(): add ~/.omp/agent/sessions as a default nested root. An env redirect via PI_CODING_AGENT_DIR dedupes against it through the existing realpath map.
  • classify(): scan the first head lines for the type:"session" entry instead of requiring it on line 1.
  • enumerate(): descend one extra level inside nested roots — omp writes subagent transcripts at <escaped-cwd>/<session-id>/<Name>.jsonl.
  • read(): accept model_change.model as a fallback for modelId (omp writes model).

Verification

Against a live omp store (~500 .jsonl across ~55 project dirs, including ~250 nested subagent transcripts): all files classify with a real cwd, and read() parses message/toolCall/toolResult events and the model unchanged — the transcript body format is identical to pi's. New fixture test/fixtures/omp-session.jsonl covers the title-record layout and model field; the enumerate test covers the omp root and the depth-3 subagent layout. pnpm run check passes (662 tests).

omp (Oh My Pi) writes the same version:3 JSONL session format as pi under
~/.omp/agent/sessions/, but prepends a fixed-width {type:"title"} record so
the {type:"session", cwd} entry is line 2, and nests subagent transcripts
one level deeper at <escaped-cwd>/<session-id>/<Name>.jsonl.

- Add ~/.omp/agent/sessions as a default nested store root (omp honors
  PI_CODING_AGENT_DIR, so an env redirect still dedupes against it).
- classify() scans the first head lines for the session entry instead of
  requiring it on line 1.
- enumerate() descends one extra level inside nested roots.
- read() accepts model_change.model as a fallback for modelId.
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with the documented omp layouts covered without weakening existing transcript validation.

Reviews (1) · Last reviewed commit: "feat(discovery): discover omp session tr..."

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate.

Reviewed head 82448d1290245b545b5aa3271d5a7267ad4141d3 (diff + fixture + prior OMP thread).

VISION verdict (per rule):

  1. Evidence is the only currency — aligns. Inspected test/fixtures/omp-session.jsonl: title line then type:session + cwd, real user/assistant/toolResult turns, model_change.model. classify()/read() keep quote-bearing turns; no summary/checkpoint path.
  2. The human owns the weights — aligns (discovery only; no write-path change).
  3. Nothing the model says is taken on faith — aligns. Pinned fixture + adapter tests (classify past title, model fallback, enumerate omp root + depth-3 subagent). Not a README-only row.
  4. The budget is the constraint — n/a (discovery).
  5. It reads what you already have — aligns. Local ~/.omp/agent/sessions, fail-soft via existing listDirs/listFiles, no upload/key. Same evidence class as pi. More harnesses welcome; folding omp into the pi adapter (identical version:3 JSONL) should not destabilise other adapters. Matches prior OMP ready-for-pr (Features: Add support for OMP "Ohmypi" #53) and feat(discovery): add OMP session discovery #21 triage (default omp discovery was not held as a captain default-behavior call).
  6. Failure is loud and named — aligns. Non-session heads still classify null and are skipped; missing store stays empty like other nested roots.
  7. Scope — aligns (adapter discovery, not a writer/provider/CI).

contract-class: restore — OMP support is already product-accepted (#53 ready-for-pr); this closes the pi-adapter gaps (store root, title-record line 2, nested subagent depth, model field) so unconfigured discovery matches that promise for pi-compatible omp transcripts. Not treating as new-default (same stance as #21).

Security: clean. Local read-only discovery; no secrets, no egress, no permission changes.

CI: first-time fork workflows approved. CI (ubuntu+macos) and Guard green on this SHA. Require no-mistakes is red — PR body has no no-mistakes attestation / signature (opened run 35051548667 failed). That check is blocking.

Still waiting on you, not on the captain: run git push no-mistakes so the body attestation matches head 82448d12. Optionally add Closes #53 in that body rewrite. Branch is ahead 1 / behind 0 of main — no rebase needed. I will not merge until no-mistakes is green on this head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants