Repository navigation
perf(provider): guide safe read-only shell batching (#28589) - #15
MattRiddell wants to merge 1 commit into
Conversation
MattRiddell
left a comment
There was a problem hiding this comment.
Independent exact-head review for board pingdotgg#28619 / pingdotgg#28589: PASS.
Reviewed #15 at d5da70d against base c3f76eb. No high, medium or low findings.
Standards: the two-file change follows the repository's smallest-change guidance. The shared instruction builder retains runtime metadata and PR-link instructions. No executable tool, installer, systemd unit, live state or service configuration changes.
Behavior and scope: known independent read-only commands may share one shell call; dependencies, inspection boundaries, mutations, approvals and permission differences remain separate. Output identity and each exit status are required. Guidance explicitly distinguishes shell-tool call savings from subprocess starts. All six existing provider paths consume the builder: Claude, Cursor, OpenCode, Antigravity and Grok directly; Codex through CodexDeveloperInstructions and CodexSessionRuntime. Tests cover the positive instruction and refusal boundaries for all six harness names.
Verification: inspected the full diff, complete changed files, provider injection call sites, repository AGENTS.md and pingdotgg#28589 packet c342275. Independently fetched the two base/head blobs and matched all four SHA256 values in that packet. Existing exact-head evidence records 12 targeted tests, scoped TypeScript and two-file lint passing. This review did not rerun tests or treat queued GitHub CI as a pass.
Performance acceptance remains OWED: the before census already met under150; guidance alone proves no reduction. After deployment, the owning lane must collect the passive 60s census and publish absolute/normalized starts, lane endpoints, system CPU and raw evidence on pingdotgg#28589, pingdotgg#24236 and pingdotgg#19810.
Acceptance applies only to this head; a changed head requires review. Ops owns merge/build/install/service window per the controller assignment and c342275. No deployment, merge or live install was performed by this reviewer.
VERDICT=PASS head=d5da70da084a07c185c2d8ef7aac8fbba849f248 findings=0
Thread transfer impact
This comment will update automatically after the next completed run. |
Problem and change
Separate shell tool calls for already-known independent reads add provider shell starts. Shared runtime instructions now prefer one shell call for those reads, while keeping dependent commands, mutations, approvals and different permissions separate. The guidance preserves command output identity and individual failure reporting. It does not automatically coalesce tool calls or change provider execution.
Board pingdotgg#28589; Matt q6206:1 assigned isolated MattRiddell/t3code source work. Live T3 state, service changes and deployment remain outside this lane's authorization.
Validation
Passive 60-second BEFORE census: 97.6 starts/s; running/ready lanes 22 to 21, mean 21.5; 4.539 starts/s/lane; system+irq+softirq 3.0292%. Raw JSON is attached to pingdotgg#28589 (53993), pingdotgg#19810 (53996) and pingdotgg#24236 (53999), SHA256 8adcc75691f671b7d62470e65e07601b45bc06c6746f21f184cfae662867e0a2. The baseline already meets the absolute target before this change; no reduction is proven.
POST-DEPLOY OWED: controller deploys the accepted source revision; owning lane repeats the passive 60-second census with endpoint lane counts, normalized starts/s/lane and the retained absolute under150 target. Post the result on pingdotgg#28589, pingdotgg#24236 and pingdotgg#19810. These tickets remain open until the required evidence exists.
Rollback: controller reverts this commit and deploys the reverted source revision through the normal T3 delivery process. No lane install or service restart.
Model: GPT-6.1 Sol. Harness: Codex through T3 Code.