feat(integrations): add provider-independent Pi/OMP extension - #2657
feat(integrations): add provider-independent Pi/OMP extension#2657gurkanguray wants to merge 39 commits into
Conversation
PR governanceThis PR follows the template and is marked ready for human review. |
0e7a2a9 to
15330a2
Compare
JerrettDavis
left a comment
There was a problem hiding this comment.
Reviewed the Pi/OMP extension end-to-end after the latest updates. The context transformation path stays copy-on-write, retrieval is hash/substring guarded, protected/recent/error tool results remain fail-open, and the runtime/client/cache boundaries look appropriately conservative for a host extension.
I pushed one maintainer cleanup commit (chore(pi-extension): fix whitespace) to clear the local git diff --check failure. No behavior changes in that commit.
Local validation:
npm testinintegrations/pi-extension-> 69 passednpm run typecheckinintegrations/pi-extension-> passednpm run buildinintegrations/pi-extension-> passednpm audit --omit=dev --jsoninintegrations/pi-extension-> 0 vulnerabilitiesnpm pack --dry-run --json-> package includes only the intended runtime/docs filesgit diff --check upstream/main...HEAD-> passed after the whitespace cleanup
GitHub is still showing only the PR governance checks queued/blocked on the fork workflow side, so this still needs the normal maintainer merge-gate handling.
|
Thanks for the review |
| ) { | ||
| return cachedCandidate; | ||
| } | ||
| if (CCR_MARKER_RE.test(originalText)) return undefined; |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
+1 |
Prepare eligible tool results asynchronously and substitute only validated cached context without rewriting provider endpoints. Preserve raw transcripts, exact CCR retrieval, protected results, and fail-open behavior.
08ea9fd to
36f7815
Compare
JerrettDavis
left a comment
There was a problem hiding this comment.
Re-approved after the CI-only repair at 36f7815. The previously reviewed feature tree is byte-identical to the approved head outside .github/workflows/pi-extension.yml; the workflow now installs --extra proxy, which supplies FastAPI before starting headroom proxy. The initial commit message was reworded only to wrap its overlong body line for commitlint.
Verification: git diff 08ea9fdfc..36f78154 -- :!.github/workflows/pi-extension.yml is empty, and all rewritten commit-message lines are within the repository’s 200-character limit.
JerrettDavis
left a comment
There was a problem hiding this comment.
The workflow/commitlint repair is correct, but the newly enabled live matrix now exposes a real product-contract blocker in both supported Pi versions.
npm run test:live fails identically for Pi 0.80.10 and 0.82.1:
validates real compression and retrieval before model-independent substitution: no prepared entry is found (entryis undefined ate2e/live-contract.test.ts:121).fails open while offline and resumes prepared substitution after recovery:waitFortimes out at line 190 after ~60 seconds.
This is no longer an installation/infrastructure failure—the canonical proxy starts successfully with [proxy] installed, and the hot-path test passes. Please reconcile the extension’s prepare/retrieve assumptions with the current Headroom CCR API, then prove both live cases pass for both matrix versions. Keep the workflow’s uv sync --frozen --extra proxy fix.
Ship the HEADROOM ASCII banner and point pi.image at a live branch URL until the file exists on main.
|
@JerrettDavis Live contract should be fixed — /v1/compress now sends config.mode: "ccr". Local test:live is 3/3. Please re-run the Pi matrix. |
Keep agent task notes out of the published branch.
JerrettDavis
left a comment
There was a problem hiding this comment.
The prior live-contract blocker is resolved on the latest source. I re-ran the extension unit suite (78 tests), typecheck, build, package dry-run, a real proxy-backed live contract (3 tests), and the focused Python lifecycle/release suites (122 tests); all passed. I also audited the native-host transaction/rollback and exact-artifact release gates.
I made one small release-metadata cleanup on the branch: the npm banner now uses the durable upstream main URL instead of the contributor feature branch. The package gates remain green after that change. This is ready from my review.
|
Design warning: strict-prefix provider cache destruction via per-request outbound rewriting Context: I maintain an OMP setup that used the third-party The failure mechanism (in omp-headroom)Its "session archive" transform re-ran on every
Why this is relevant to this PRThe new provider-independent extension promises to "substitute only previously prepared results in the model-facing context, and preserve the host session transcript byte-for-byte" — that is the right shape, and it avoids the whole-history rewrite bug by construction. The remaining risk is narrower but still real:
I'd be happy to provide the full measured dataset (per-request cache_read curves for healthy vs broken windows) if useful for the PR's test design. |
chopratejas
left a comment
There was a problem hiding this comment.
Holding this one out of the current merge batch — not on quality, but because there is an open bug filed directly against this branch: #3056, "Pi extension double-compresses RTK-truncated tool results."
The reporter ran this PR end to end and measured the failure: 262,369 chars → 12,000 via RTK → 10,698 via Headroom. The candidate policy in plugins/pi/src/policy.ts treats an already-truncated toolResult as compressible because the pruning-metadata check doesn't look at the RTK markers. Compressing a compaction is not just wasted work — it degrades content the upstream optimizer already decided to keep.
Concretely, the metadata check needs to recognise both shapes the reporter observed:
details.rtkCompaction.truncated
details.metadata.rtkCompaction.truncated
They also specified the regression test they'd want, which is worth taking as written.
Everything else here looks ready, so I'd rather land it once with the guard than land it and immediately follow with a fix. Once #3056's case is covered and there's a test pinning it, this is good to go from my side.
Separately: CI on this branch has only run 2 checks, which is well short of the ~35 a full run produces. Worth pushing an empty commit to re-trigger so the merge decision rests on a complete run.
|
Thanks @chopratejas #3056 is fixed and covers both RTK metadata paths, with regression tests. Post-sync validation also passed with 91 plugin tests, 276 Python tests, build/typecheck, audit, and the full Pi/OMP matrix. About CI, GitHub still shows only two governance checks after my commits. I see |
|
Hi @dio16, can you please provide the dataset? Also, I've been using this pr for a while and haven't faced this issue yet. I need to observe this more closely by reproducing it locally |
|
Hi @gurkanguray — here's the dataset from the omp-headroom investigation (referenced in DarkPhilosophy/omp-headroom#2). Full per-request curves + README: https://gist.github.com/dio16/e1a9b1b5bc1eba21d1a3ef8c9e44c16e Same binary and provider/model across both windows ( Broken window — session
Healthy window — 08-02..08-08, 1,082 deepseek-v4-flash requests:
Post-fix A/B (fresh process, Strict-prefix sensitivity (session Why you likely haven't seen it with this PR: the rewrite lived in omp-headroom's archive layer, which recomputed a new Reproduction recipe: |
|
Thanks for sharing the dataset @dio16 . The failure came from |
JerrettDavis
left a comment
There was a problem hiding this comment.
Re-reviewed and refreshed the current head at eb5e1ba. The RTK guard now recognizes both documented metadata paths (including mixed direct/nested values) and keeps malformed/non-truncated metadata eligible, so it avoids double-compressing an upstream-truncated result without broad false positives. I merged current main, resolved release metadata by keeping the 0.36.0 fleet and adding headroom-pi at 0.36.0, and aligned the Pi package/lock versions. A Windows-only mypy defect in the POSIX fcntl branch was fixed with a typing-only Any cast matching the existing msvcrt pattern.
Validation: 91 extension tests, TypeScript typecheck, 53 passed/1 skipped Pi lifecycle Python tests, production npm audit (0 vulnerabilities), version alignment, Ruff, and full commit hooks including mypy all pass. No remaining source blocker; fork CI may still require workflow authorization.
Description
Add one provider-independent Headroom extension for Pi and Oh My Pi. The extension prepares eligible large tool results asynchronously, substitutes only validated cached output in the copied model-facing context, and preserves the raw host transcript.
This is a local-install fleet path: one loopback proxy plus
headroom-pi. It coexists with the existing Anthropic inference-proxy wrapper. Pi wrap does not rewrite inference URLs. OMP wrap still ownsmodels.yml; durable OMP setup never touches that file.Closes #2655
Type of Change
Changes Made
headroom-piatplugins/pi(same layout as OpenClaw/OpenCode). One package loads in Pi and OMP./v1/compressnow sendsconfig.mode: "ccr"so prepare/retrieve matches the current Headroom CCR API (review 4921293436).{status:"skipped",reason:"noop"}/{status:"rejected",reason}. No-ops are skipped, not rejected.lastErrorclears after a later accept.Headroom saved N tokens this session, persisted next to the Pi session file across/reloadand/resume./newresets it.headroom init -g pi/headroom init -g omppins the exact unpublished-safe release version after npm publish.headroom wrap pisetsHEADROOM_PI_BASE_URLonly.headroom/providers/pi_extension.py. There is noproviders/pi/inference rewrite./metricsscrape failures return 500 with# scrape_error ...andheadroom_metrics_scrape_errors_total.headroom doctorprobes/metricsand the native pin.headroom-pibefore PyPI. CI matrix: Pi0.80.10/0.82.1/0.84.1, OMP17.1.8. Workflow still usesuv sync --frozen --extra proxy.plugins/pi/README.md,wiki/cli.md,wiki/pi-omp-runbook.md.Testing
pytest) — focused Python suites below.ruff/mypyrerun after the main merge.npm test).npm run typecheck).Test Output
Real Behavior Proof
v24.12.0, checkout Headroom0.35.0-devafter merge withorigin/main, loopback proxy on:8787.HEADROOM_PI_BASE_URL=http://127.0.0.1:8787 npm run test:liveinplugins/pi. Payload includesconfig.mode: "ccr". Both live cases from the review (validates real compression and retrieval...andfails open while offline and resumes...) pass.plugins/pimove): Pi0.80.10/0.82.1and OMP17.1.8loaded the packed package; raw history unchanged; model switch reused the prepared result. That matrix was not re-run after the path rename + main merge.headroom init -g(headroom-piis still npm 404); real external model accounts; non-loopback Headroom; upstream fork workflows (need maintainer approval).Runtime Rollout Safety
headroom init -g remove piand/orheadroom init -g remove omp. Or stop the local proxy.headroom-aiand rerunheadroom init -g pi/headroom init -g ompso the pin matches that CLI. Do not installlatest.Review Readiness
Maintainer gates still pending: core-maintainer approval of #2655, permission to run fork workflows, and first
headroom-pinpm publish before durableinit -gcan pin a released version.Checklist
CHANGELOG.md— the PR title supplies the release-please entry.Screenshots (if applicable)
N/A — terminal extension and host lifecycle change.
Additional Notes
plugins/pi/src/bridge.ts:buildCompressPayloadsendsconfig: { compress_user_messages: false, mode: "ccr" }. The workflow still usesuv sync --frozen --extra proxy.HEADROOM_PI_BASE_URLonly. Package install/ownership isheadroom/providers/pi_extension.py, not aproviders/pi/inference analog.headroom-pipublish.