Repository navigation
fix(trace): separate Pi cache token accounting - #3719
Open
waveywaves wants to merge 1 commit into
Open
waveywaves wants to merge 1 commit into
waveywaves wants to merge 1 commit into
Conversation
Recompute Pi v2 totals from native input and output counters, preserve cache categories, and verify them against cache-active raw session records. Assisted-by: Vibhav's Pi Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
Contributor
PR validation —
|
| Status | Policy | Material | Messages |
|---|---|---|---|
pr-min-approvals |
pr-info |
PR/MR #3719 has 0 approving reviews, 1 required. | |
| ✅ Passed | pr-description-required |
pr-info |
- |
| ✅ Passed | pr-user-story-linked |
pr-info |
- |
AI Session Checks — ⚠️ no AI session found
Missing AI Coding Sessions
This organization requires every PR to be backed by a Chainloop Trace AI coding session, and none was found for this one.
Please make sure the AI coding session evidence has been sent by the Chainloop CLI, or add the skip-ai-session label to this PR to bypass this check.
Learn more about Chainloop Trace.
Security Checks — ✅ 6 passing
✅ secret-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | secrets-detection |
- |
✅ sast-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | cwe-top25 |
- |
| ✅ Passed | cwe-top26-40-cusp |
- |
| ✅ Passed | owasp-top10-2025 |
- |
| ✅ Passed | sast |
- |
✅ iac-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | iac-misconfiguration |
- |
Scans not applied (2)
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
Security context
✅ Nothing this change touches has a recorded security-fix history.
View in Chainloop ↗ · How this works ↗
Powered by Chainloop and Chainloop Trace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This was referenced Oct 24, 2026
migmartri
reviewed
Oct 26, 2026
| # Codex token accounting | ||
|
|
||
| Codex and Claude Code evidence use disjoint token categories: | ||
| Codex v2, Pi v2, and Claude Code evidence use disjoint token categories: |
| | Codex `v1` | Input includes cache; total includes input and output. | Use `total_tokens` without adding the cache fields. Counts may still contain the old overcount. | | ||
| | Codex `v2` | Input and total exclude cache, matching the existing Claude convention. | Add both cache fields to `total_tokens` exactly once. | | ||
| | Pi `v1` | Input is separate; total was supplied by Pi and may include cache. | Recompute from native input, output, and cache fields; do not assume supplied totals are consistent. | | ||
| | Pi `v2` | Input and total exclude both cache categories. | Add both cache fields to `total_tokens` exactly once. | |
Member
There was a problem hiding this comment.
I am confused, is there a v2 for pi?
| return err | ||
| } | ||
| case pi.Name: | ||
| if d.SchemaVersion != "v2" { |
Member
There was a problem hiding this comment.
is schemaversion the default one? since what version of pi?
We might want to also support this one if it's the default
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
schema_version: v2evidence with native input, output, cache-read, and cache-write counters kept separatetotal_tokensfrom base input plus output instead of trusting Pi's cache-inclusivetotalTokensThis implements the Pi-only #3720, a slice of the broader cross-provider #3706. The focused tracker was created after this PR opened to keep subsequent work issue-first. This PR does not close the broader all-provider tracker.
Trace provider checklist
Validation
go test -race -shuffle=on -count=1 ./internal/trace/pi ./internal/trace/e2ego build ./...go vet ./...golangci-lint run --new-from-rev upstream/mainCHAINLOOP_E2E_HARNESS=Pi CHAINLOOP_E2E_MODEL=openrouter/anthropic/claude-haiku-5.5 CHAINLOOP_E2E_FEATURE='Cache token accounting' make -C app/cli test-trace-e2eThe live run had nonzero cache reads and writes and passed with the matrix expecting
Yes.make -C app/cli teststill hits two existing local-environment failures that reproduce unchanged onupstream/main:cmd/TestLoadAuthTokenwhenCHAINLOOP_TOKENis inherited, and CodexTestCopySessionData*on the macOS storage-root path. The changed Pi and E2E packages pass under the race detector.AI assistance
Vibhav's Pi assisted with implementation, testing, and review. The commit is signed, DCO-compliant, and carries
Assisted-by: Vibhav's Pi.The
skip-ai-sessionlabel is required because the available Chainloop login cannot access thechainloopproject to upload the session. This matches the fallback used on merged Pi PR #3704; it does not bypass code, test, DCO, or signature checks.