WIP: Laminar tracing groundwork (pending clarification) #8325
Conversation
- Introduce LaminarService (lightweight, no-op if disabled)\n- Start span in Task.submitUserMessage() and end in attemptCompletionTool()\n- Add types shim at src/types/lmnr.d.ts to avoid missing module types\n- Tests pass (cd src && npx vitest run)\n- Lint passes (pnpm run lint)\n\nNote: groundwork for porting cline/cline#5862; pending clarification on full scope.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| * Safe no-op when disabled; still returns an id for balanced calls. | ||
| */ | ||
| public startSpan(name: string, attributes: Attributes = {}): string { | ||
| const id = `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}` |
There was a problem hiding this comment.
P1: Not a true no-op when disabled. startSpan/endSpan record spans and debugSpan logs even if initialize() never enables tracing. Gate behavior behind enabled (bail early in startSpan/endSpan; guard debug/debugSpan) to avoid console noise and memory overhead.
| // Try to dynamically load the SDK if enabled | ||
| if (this.enabled) { | ||
| try { | ||
| const mod = await import("@lmnr-ai/lmnr") |
There was a problem hiding this comment.
P2: Dynamic import likely needs default handling. If the SDK exports default, future calls like this.sdk.init may fail. Normalize the module:
| const mod = await import("@lmnr-ai/lmnr") | |
| const mod = await import("@lmnr-ai/lmnr"); | |
| this.sdk = (mod as any).default ?? mod; |
|
|
||
| // End current task.step span if active | ||
| try { | ||
| if ((cline as any).laminarSpanId) { |
There was a problem hiding this comment.
P2: Accessing a private field via (cline as any).laminarSpanId bypasses types and couples modules. Prefer a Task instance method to end the active span (or encapsulate span lifecycle within Task).
|
|
||
| // Start a tracing span for this user turn | ||
| try { | ||
| this.laminarSpanId = laminar.startSpan("task.step", { taskId: this.taskId }) |
There was a problem hiding this comment.
P2: Span starts here but ends in attemptCompletionTool. If control flow exits early or errors differently, spans can remain open. Consider centralizing lifecycle in Task (e.g., try/finally) or using a Task event hook to end the span reliably.
|
|
||
| // Simple in-memory span store and active stack | ||
| private spans = new Map<string, InternalSpan>() | ||
| private activeStack: string[] = [] |
There was a problem hiding this comment.
P1: activeStack is process-global; concurrent tasks may interleave and corrupt parent/child relationships. Use AsyncLocalStorage for per-async-context stacks or maintain stacks keyed by sessionId/taskId.
| private debug(msg: string) { | ||
| // Keep logs minimal to avoid noise | ||
| try { | ||
| console.log(msg) |
There was a problem hiding this comment.
P3: console.log in debug() adds noise and may violate lint rules. Prefer console.debug or a central logger, and gate logs behind an opt-in debug flag and/or enabled.
| @@ -0,0 +1,4 @@ | |||
| declare module "@lmnr-ai/lmnr" { | |||
| const mod: any | |||
There was a problem hiding this comment.
P3: Type shim assumes a default export; if the SDK migrates to named exports this could mislead consumers. Consider exporting both default and named to keep flexibility.
hannesrudolph
added
PR - Draft / In Progress
and removed
Issue/PR - Triage
This PR introduces a minimal LaminarService wrapper and wires basic task.step span start/end around user messages and attempt_completion.\n\nChanges:\n- Added LaminarService (safe no-op wrapper with future SDK hook) at src/services/laminar/LaminarService.ts\n- Start span in Task.submitUserMessage() and end in attemptCompletionTool()\n- Add type shim src/types/lmnr.d.ts to avoid missing module type error\n- Tests pass: cd src && npx vitest run\n- Lint passes: pnpm run lint\n\nThis is groundwork pending clarification for full port of cline/cline#5862.