Skip to content

feat(learn): collect Pi sessions alongside other agents - #1239

Open
tranducquy0 wants to merge 4 commits into
JuliusBrussee:mainfrom
tranducquy0:feat/learn-collect-pi-sessions
Open

tranducquy0 wants to merge 4 commits into
JuliusBrussee:mainfrom
tranducquy0:feat/learn-collect-pi-sessions

Conversation

@tranducquy0

Copy link
Copy Markdown
Contributor

Adds a pi learn session source that discovers and scans Pi agent session JSONL files, and includes Pi in the default scan sources, source line, empty-state text, and --sources usage.

  • New piSessionSource in proxy/internal/store/source_pi.go with usage/cache/billing/tool-call extraction
  • piRoot() config (honors CAVEMAN_PI_ROOT, defaults to ~/.pi)
  • Registered in learnSessionSources() and default source list
  • CLI copy + --sources help updated

Comment thread proxy/internal/store/source_pi.go Fixed
@tranducquy0

Copy link
Copy Markdown
Contributor Author

Updated in two follow-up pushes:

  • 9516db3b fix(learn): use a constant upper-bound check (total > math.MaxInt) for the Pi context conversion in proxy/internal/store/source_pi.go, resolving the CodeQL go/incorrect-integer-conversion finding — CodeQL only recognizes bounds checks against a known constant, not uint64(^uint(0)>>1).
  • aee5c9f1 test(learn): update learn-porcelain.runtime.mjs and learn-tui.runtime.mjs expected empty-source message to include opencode, aider or Pi.

generated by OpenCode

Copy link
Copy Markdown
Owner

Reviewed, not adopted — needs a test and two usage-arithmetic fixes. Thanks for this; adding a learn source is real work and the hard part is right.

Verified in your favour: the session path is correct. pi 1.1.0's own bundle writes ~/.pi/agent/sessions/<encoded-cwd>/, which is exactly what piRoot() + filepath.Join(root, "agent", "sessions") walks. The CAVEMAN_PI_ROOT override and repoProvisional handling match the sibling sources.

What needs changing:

  1. No test. Every sibling has one — source_{claude,codex,gemini,opencode,aider}_test.go. 225 lines of new parsing with no fixture is the main blocker; this is also the repo's standing rule that a fix needs a test.
  2. piCacheUsage and piBillingUsage sum two spellings of one field rather than preferring one: usage["cache_read"] + usage["cacheRead"], usage["input"] + usage["input_tokens"], and the same for cache_write/cacheWrite/cache_creation/cacheCreation. pi's bundle uses both conventions (cacheRead 4279×, cache_creation 40×, cacheCreation 9×), so a payload carrying both double-counts tokens. Prefer first-present, as piContextTotal already does.
  3. piTaskSpawns counts every toolCall block as a spawn, so an ordinary tool-using turn reports N spawns. The siblings count subagent/Task spawns only.

Minor: piToolCalls fills the pending map with tool-result text that nothing ever reads, so Pi tool output never reaches the analysis the other sources feed.

Fix 1–3 and this is straightforwardly reviewable — the structure is already right.


Generated by Claude Code

This branch has not been deployed

No deployments
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.

3 participants