Skip to content

chore: simplify langfuse module + multi-step coordinator#43

Merged
iskakaushik merged 1 commit intomainfrom
chore/simplify-langfuse-and-coordinator
May 1, 2026
Merged

chore: simplify langfuse module + multi-step coordinator#43
iskakaushik merged 1 commit intomainfrom
chore/simplify-langfuse-and-coordinator

Conversation

@iskakaushik
Copy link
Copy Markdown
Collaborator

Summary

Cleanup pass over the langfuse + multi-step coordinator code that just landed (#40, #42). No behaviour changes for the public API; simplifications and dependency hygiene.

Langfuse module

  • One ISO-8601 helper (Trace::to_iso8601(time_point)) replaces three duplicated `gmtime_r`/`snprintf` blocks (the named static plus two inline lambdas in `finish_generation` / `build_trace_event`).
  • Capture timestamp once per span event instead of two separate clock reads.
  • Magic-string constants in the anonymous namespace for the Langfuse event type discriminators (`trace-create` / `span-create` / `span-update` / `generation-create`) and for level / unit strings.
  • Shared `make_span_create` + `wrap_event` helpers so the orphan-span fallback in `record_tool_call_finish` reuses them instead of duplicating the body-construction.
  • Drop over-engineered `ParsedHost`: `httplib::Client`'s URL constructor handles scheme/host/port already; we only need a small `extract_base_path` for Langfuse instances served under a sub-path.
  • Cache `httplib::Client` + Basic-auth headers on `Tracer` (lazily constructed under `mu_`) so repeat `send_batch` calls reuse the TLS session.
  • Replace 6-arg `start_trace` with `start_trace(name, TraceOptions{})` to match the rest of the SDK's struct-of-options style.
  • Strict by default: `Config::error_policy = ErrorPolicy::kStrict` (was `best_effort = true`) so misconfigurations surface at integration time instead of being silently swallowed.
  • Reject post-end mutations: `set_` / `instrument` / `finish_generation` / `record_` bail out once `Trace::end()` has fired.
  • Replace hand-rolled UUID v4 with stduuid (header-only, MIT, vendored under `third_party/stduuid-header-only/`). Wired into `ai-sdk-cpp-langfuse` via a PRIVATE link.

Multi-step coordinator

  • Avoid re-copying `initial_messages` every loop iteration. `step_messages` is grown in place: `erase` back to the immutable prefix + `insert` the running `response_messages` accumulator, instead of `step_options = initial_options; step_options.messages = initial_messages; step_options.messages.insert(...)` per step.
  • Remove dead `create_next_step_options` stub from the public header + impl (not part of any external ABI; was only kept as scaffolding during the refactor).
  • Trim a narrating comment that referenced an external repo path.

Test plan

  • `make ai_tests langfuse_tracing` clean
  • Full `ctest` — 221/227 pass; the 6 failures are `ClickHouseIntegrationTest.*` cases that need a local ClickHouse server (unchanged from `main`)
  • End-to-end `langfuse_tracing_debug` against `us.cloud.langfuse.com` — trace ingested with 1 generation + 2 tool spans nested correctly under it
  • Multi-step duplicate-execution suite — 4/4 pass for both providers

Langfuse module (`include/ai/langfuse.h`, `src/langfuse/tracer.cpp`):
- Single Trace::to_iso8601(time_point) helper replaces three
  duplicated gmtime_r/snprintf blocks (now_iso8601 plus two inline
  lambdas in finish_generation / build_trace_event).
- Capture timestamp once per record_tool_call_{start,finish} event
  (previously two clock reads per span event).
- Anonymous-namespace constants for event type discriminators
  (trace-create / span-create / span-update / generation-create) and
  level / unit strings, replacing scattered magic strings.
- Extract make_span_create + wrap_event helpers; the orphan-span
  fallback in record_tool_call_finish reuses them instead of inlining
  20+ lines of duplicated body construction.
- Drop the over-engineered ParsedHost/parse_host: httplib::Client's
  URL constructor handles scheme/host/port already; we only need a
  base-path extractor for Langfuse instances served under a sub-path.
- Cache httplib::Client + Basic-auth headers on the Tracer (lazily
  constructed under mu_) so repeat send_batch calls reuse the
  connection / TLS session instead of paying a fresh handshake.
- Replace 6-arg `start_trace(name, input, user, session, metadata,
  tags)` sprawl with `start_trace(name, TraceOptions{})` to match the
  rest of the SDK's struct-of-options style.
- Default Config::error_policy = ErrorPolicy::kStrict (was
  best_effort=true) so misconfigurations surface at integration time
  instead of being silently swallowed.
- Reject post-end mutations: set_*/instrument/finish_generation/record_*
  bail out if Trace::end() has fired.
- Replace hand-rolled UUID v4 with stduuid (vendored under
  third_party/stduuid-header-only/).

Multi-step coordinator (`src/tools/multi_step_coordinator.cpp`):
- Avoid re-copying initial_messages every loop iteration. step_messages
  is grown in place: erase back to the immutable prefix + insert the
  running response_messages accumulator, instead of `step_options =
  initial_options; step_options.messages = initial_messages;
  step_options.messages.insert(...)` per step.
- Remove the dead create_next_step_options stub from header + impl
  (not part of any external ABI; was only kept as scaffolding during
  the refactor).
- Trim narrating comment that referenced an external repo path.

Vendoring:
- Add third_party/stduuid-header-only/ (uuid.h + LICENSE) and
  third_party/stduuid-cmake/ wrapper exposing a stduuid::stduuid
  INTERFACE target. Wired into ai-sdk-cpp-langfuse via PRIVATE link.

Verified: full ctest suite (221/227 pass; 6 failing are
ClickHouseIntegrationTest cases that need a local ClickHouse server,
unchanged from main). End-to-end Langfuse example produces the
expected trace with 1 generation + N tool spans nested correctly.
@iskakaushik iskakaushik merged commit 09c423c into main May 1, 2026
2 checks passed
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.

1 participant