Skip to content

feat: add capture_ai() on an isolated AI capture lane - #250

Merged
eli-r-ph merged 4 commits into
v1from
eli.r/capture-ai-lane
Sep 9, 2026
Merged

eli-r-ph merged 4 commits into
v1from
eli.r/capture-ai-lane

Conversation

@eli-r-ph

@eli-r-ph eli-r-ph commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Adds capture_ai() so Rust callers can send LLM analytics events to the live POST /i/v1/ai/events endpoint. AI events ride their own isolated transport lane, so a multi-MB AI event can never delay or displace analytics events. Targets v1; all public API changes are additive.

  • Adds capture_ai, capture_ai_batch, capture_ai_immediate, capture_ai_batch_immediate, and the global posthog_rs::capture_ai, mirroring the analytics entry points.
  • Adds Endpoint::CaptureAi and CaptureFailure::endpoint() so an on_error hook can tell which lane failed.
  • Adds capture_ai_compression (mirrors capture_compression; unset sends AI bodies uncompressed, zstd recommended) and capture_ai_max_queue_size (default 1000).
  • The AI lane has its own worker, queue, and retry state, started on the first capture_ai* call, and drained by flush/shutdown/Drop alongside the analytics lane.
  • The AI lane batches by serialized bytes (5 MiB soft target) and drops events whose properties exceed the backend's 8 MiB ceiling locally, logging only the event name and size.
    • The immediate variants send oversize events instead and return the backend's ai_event_too_big verdict.
  • Neither capture nor capture_ai inspects the event name; the backend decides routing and reports per event, and that verdict already reaches on_error.
  • A 2xx with per-event drops and no on_error hook now logs one aggregate warning per batch on both lanes; it was silent before.
  • The analytics lane is unchanged: the worker and pipeline are parameterized by a lane config whose analytics variant reproduces the previous behavior, and the existing tests pass unmodified.

🤖 Generated with Claude Code

Add `capture_ai`, `capture_ai_batch`, `capture_ai_immediate`,
`capture_ai_batch_immediate`, and the global `capture_ai`, posting to
`POST /i/v1/ai/events` on a second transport lane: its own worker thread,
queue, and retry state, started on the first call. The lane pins zstd,
batches by serialized bytes (5 MiB soft target, guarded check-before-append),
and pre-drops events whose properties exceed the backend's 8 MiB ceiling,
logging only the event name and byte size. The SDK never tests the event
name; the backend's per-event verdicts reach `on_error`, which now reports
the lane via `CaptureFailure::endpoint()`. A 2xx with per-event drops and no
hook registered now logs one aggregate warning per batch on both lanes.

The analytics lane is unchanged: the worker and pipeline are parameterized
by `LaneConfig`, and `LaneConfig::analytics()` reproduces prior behavior.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@eli-r-ph eli-r-ph self-assigned this Sep 8, 2026
@eli-r-ph
eli-r-ph marked this pull request as ready for review September 8, 2026 23:18
@eli-r-ph
eli-r-ph requested a review from a team as a code owner September 8, 2026 23:18
@eli-r-ph
eli-r-ph requested review from a team September 8, 2026 23:18
Comment thread src/client/transport.rs Resolved
@veria-ai

veria-ai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
### Issue 1
src/client/async_client.rs:180-185
**Shutdown races AI startup**

A first `capture_ai` call can observe the analytics transport as open just before `shutdown` snapshots the existing lanes. If shutdown closes only the analytics lane, `get_or_init` can then create a new AI transport with its own open state. The event is delivered after shutdown returns, and the new worker remains alive until `Drop`, violating the shutdown contract. The blocking client has the same race at `src/client/blocking.rs:183-188`; lazy initialization and lane snapshots need a shared lifecycle synchronization point.

### Issue 2
src/client/transport.rs:1080-1089
**Envelope bypasses request limit**

The 8 MiB admission check covers only serialized `properties`, while fields such as `event`, `distinct_id`, and group values are unbounded. An event with small properties but a large envelope can therefore pass this check and be sent alone even when its complete body exceeds the endpoint's 20 MiB request cap. This makes the documented request-size margin unreliable and can cause the background event to be rejected. Please add a total serialized-size guard or otherwise handle this case, with coverage using a large non-properties field.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat: add capture_ai() on an isolated AI..." | Re-trigger Greptile

Comment thread src/client/async_client.rs Resolved
Comment thread src/client/transport.rs Resolved
Add `capture_ai_compression` (mirrors `capture_compression`: unset sends AI
bodies uncompressed, the setter takes one of the four codecs, zstd
recommended) and `capture_ai_max_queue_size` (default 1000) so the AI lane's
codec and queue depth are configured, not hardcoded; `capture_compression`
and `max_queue_size` no longer touch the AI lane.

Close the shutdown race: `ai_transport()` re-checks the analytics lane's
closed mark after `get_or_init` and closes a lane spawned in the window, so a
first `capture_ai` racing `shutdown` cannot leave a worker delivering after
shutdown returns.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@marandaneto marandaneto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advisory review: two reproduced behavioral defects. Existing library and AI integration tests pass for both client flavors.

Comment thread src/client/transport.rs Resolved
Comment thread src/client/blocking.rs Resolved
@marandaneto
marandaneto requested a review from a team September 9, 2026 08:24
… reading lanes on shutdown

Two review findings on the AI lane. `admit_live` now owns `buffer_since`, so
an event carried into a fresh batch keeps an interval timer instead of waiting
for the next threshold or flush. `shutdown` and `Drop` mark the analytics lane
closed before reading the AI lane, so a first `capture_ai` racing shutdown is
either drained by that shutdown or closes itself in the post-init re-check;
previously the read came first and a lane initialized in between stayed open.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread src/client/async_client.rs Resolved
Comment thread .sampo/changesets/capture-ai-lane.md Resolved
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@eli-r-ph
eli-r-ph merged commit c355b18 into v1 Sep 9, 2026
29 checks passed
@eli-r-ph
eli-r-ph deleted the eli.r/capture-ai-lane branch September 9, 2026 18:51
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.

2 participants