perf(server): drop transient native events from provider logs - #240
perf(server): drop transient native events from provider logs#240leoisadev1 wants to merge 1 commit into
Conversation
Streaming provider chunks were filling thread-scoped ndjson files even though they cannot change thread state. Filter those native events before serialization, push into the pending buffer instead of copying it, and keep ACP protocol logging opt-in with the same chunk filter. Async drain/close for EventNdjsonLogger stays on OPEN #203. Grok 4.6 High via Grok Build.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis change reduces persisted provider log volume and makes ACP protocol diagnostics conditional. One non-blocking operational gap remains: Grok and Cursor do not expose or forward the setting required to enable full ACP protocol logging, so operators cannot turn on the documented diagnostics in production. Confidence Score: 4/5Safe to merge from a correctness and security perspective, but production ACP protocol diagnostics remain unavailable to operators until configuration is wired through the Grok and Cursor adapters. The executed check confirmed one limited operational configuration gap. No runtime failure, data-integrity issue, or security issue was established. Files Needing Attention: apps/server/src/provider/acp/AcpNativeLogging.ts; apps/server/src/provider/Layers/GrokAdapter.ts; apps/server/src/provider/Layers/CursorAdapter.ts; packages/contracts/src/settings.ts
What T-Rex did
|
| payload: formatRequestLogPayload(event), | ||
| }), | ||
| ...(input.nativeEventLogger | ||
| ...(input.nativeEventLogger && input.verboseProtocolLogging |
There was a problem hiding this comment.
Protocol opt-in is unreachable
Full ACP protocol logging now requires verboseProtocolLogging, but the Grok and Cursor production adapters do not pass that option and their provider settings do not expose it. Operators therefore cannot enable the documented diagnostics outside direct test construction. Wire the setting through provider configuration and adapter construction. This is a non-blocking operational concern, but it leaves incident diagnostics unavailable when they are needed.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
- Authored TypeScript check reads the production ACP logging, Grok/Cursor adapter and driver, and settings schema sources and asserts that the required option is gated but never supplied or configurable; the takeaway is that the check directly covers the claimed integration path.
- Executed `pnpm exec vp test run apps/server/src/provider/acp/AcpNativeLogging.test.ts` from the repository root and captured 7 passing tests; the takeaway is that unit tests validate explicit test-only opt-in but do not establish production plumbing.
- Executed the authored reachability check from the repository root and captured its zero-exit assertion results; the takeaway is that Grok and Cursor production construction and configuration cannot set the gate's required option.
What Changed
Provider event logs no longer persist streaming chunks that cannot change thread state: Codex item/realtime deltas, Claude content-block deltas, ACP
agent_message_chunk/agent_thought_chunk, and OpenCode text/reasoning part updates. Writes push into the pending buffer instead of copying it. ACP request diagnostics stay on; full protocol logging is opt-in and applies the same chunk filter.Why
Native streams currently write every delta into thread-scoped ndjson files. Canonical events already drop
content.deltaand similar types. Native events did not, so the files grew with records nobody reads. Remaining pingdotgg#8187 logging work, kept off #230 (narrow projection queries) and off #203 (async log drains).EventNdjsonLogger.tsis shared with OPEN #203. This PR only changes persist filters and pending-buffer mutation. It does not rewrite drain/close.Out of scope here: OpenCode adapter text merge, native telemetry sampling, GitVcsDriver Trace2 decode, RepositoryIdentityResolver root cache (already local).
UI Changes
None.
Checklist
Verification
vp test run apps/server/src/provider/Layers/EventNdjsonLogger.test.ts apps/server/src/provider/acp/AcpNativeLogging.test.ts— 22 passedvp linton the five changed files — cleanvp run --filter akeru-bot typecheck— suggestions only, no errorsGrok 4.6 High via Grok Build.