feat(observability): add OTEL instrumentation, upgrade sdk-node to 0.202.0 and enforce OTel v2.x trace deps - #886
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces comprehensive observability and telemetry infrastructure to the NeuroLink SDK. It adds 11 exporter implementations, metrics aggregation with time-windowing, span serialization/processing pipelines, sampling and retry policies, and instruments key operations across providers, RAG, workflow, memory, and media handlers. New CLI commands expose observability status, metrics, and cost tracking via Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant SDK as NeuroLink SDK
participant Storage as AsyncLocalStorage<br/>(Metrics Context)
participant Provider as AI Provider
participant Aggregator as MetricsAggregator
participant Exporter as ExporterRegistry
Client->>SDK: generate(request)
SDK->>Storage: run(metricsTraceContext)
activate Storage
SDK->>Aggregator: recordSpan(rootSpan)
SDK->>Provider: generate()
activate Provider
Provider->>Provider: createSpan(MODEL_GENERATION)
Provider->>Aggregator: recordSpan(providerSpan)
Provider-->>SDK: result + usage
deactivate Provider
SDK->>Aggregator: recordSpan(finalSpan)<br/>with tokens/cost enriched
SDK->>SDK: emit generation:end event
SDK->>Aggregator: recordMemorySpan()<br/>(if memory involved)
deactivate Storage
Client->>SDK: getMetrics()
SDK->>Aggregator: getMetrics()
Aggregator-->>Client: MetricsSummary<br/>(latency, tokens, cost, traces)
Client->>SDK: getTelemetryStatus()
SDK->>Exporter: getExporterRegistry()
Exporter-->>Client: health status + config
sequenceDiagram
participant CLI as CLI User
participant Parser as yargs Parser
participant Handler as Telemetry Handler
participant SDK as NeuroLink
participant Exporter as ExporterRegistry
CLI->>Parser: telemetry status
Parser->>Handler: execute TelemetryStatusHandler
Handler->>SDK: getTelemetryStatus()
SDK->>Exporter: getExporterRegistry().healthCheckAll()
Exporter-->>SDK: health map{exporter→status}
SDK-->>Handler: {enabled, langfuse, openTelemetry, exporters[]}
Handler->>Handler: format output(text/json/table)
Handler-->>CLI: rendered status + exporter health
CLI->>Parser: telemetry stats --detailed
Parser->>Handler: execute StatsHandler
Handler->>SDK: getMetrics()
SDK->>SDK: compute latency percentiles,<br/>cost by provider/model,<br/>token usage
SDK-->>Handler: MetricsSummary
Handler->>Handler: enrichWithDetailedFields<br/>(std-dev, p75/p90/p99)
Handler-->>CLI: formatted metrics table
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR expands NeuroLink’s observability stack and instruments multiple execution paths (workflow, RAG, MCP, server middleware, TTS, PPT, evaluation) with span creation/recording, while also updating Google/Gemini model constants and some landing OG-image generation logic.
Changes:
- Introduces a new
src/lib/observability/*module (span serializer/types, exporters, sampling, processors, OTel bridge) and exports it via the library barrel. - Adds span recording across workflow runner, ensemble/judge scoring, RAG pipeline/search/prepare, MCP transport utilities, server middleware/adapter lifecycle, TTS/PPT/evaluation paths.
- Updates Google model/pricing data and enhances Vertex Gemini native path with conversation-history injection + global-endpoint systemInstruction workaround.
Reviewed changes
Copilot reviewed 79 out of 98 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/workflow/core/workflowRunner.ts | Adds WORKFLOW spans for run + streaming run paths |
| src/lib/workflow/core/judgeScorer.ts | Adds WORKFLOW judge spans and duration recording |
| src/lib/workflow/core/ensembleExecutor.ts | Adds WORKFLOW ensemble spans and success/error status logic |
| src/lib/utils/ttsProcessor.ts | Adds TTS spans around synthesis path and error recording |
| src/lib/utils/pricing.ts | Adds Gemini pricing + Vertex→Google pricing fallback |
| src/lib/types/providers.ts | Adds _traceContext on AIProvider for span hierarchy |
| src/lib/types/conversationMemoryInterface.ts | Adds optional close() lifecycle hook |
| src/lib/services/server/ai/observability/instrumentation.ts | Tweaks OTEL init to update config on re-init |
| src/lib/server/middleware/common.ts | Records spans for timing + error-handling middleware |
| src/lib/server/abstract/baseServerAdapter.ts | Records spans for server initialize + graceful shutdown |
| src/lib/rag/retrieval/hybridSearch.ts | Adds RAG search spans + counts attributes |
| src/lib/rag/ragIntegration.ts | Adds RAG “prepare” span wrapper around internal prepare |
| src/lib/rag/pipeline/RAGPipeline.ts | Adds RAG pipeline span + provider-call timeouts |
| src/lib/providers/googleVertex.ts | Injects conversation history into native Gemini path; global endpoint systemInstruction workaround |
| src/lib/providers/amazonSagemaker.ts | Refactors imports and threads neurolink into provider ctor |
| src/lib/observability/utils/spanSerializer.ts | New SpanSerializer + format conversions (Langfuse/LangSmith/OTel) |
| src/lib/observability/utils/index.ts | Barrel export for observability utils |
| src/lib/observability/types/spanTypes.ts | New span enums/types and attribute schema |
| src/lib/observability/types/index.ts | Barrel for observability types |
| src/lib/observability/types/exporterTypes.ts | New exporter config + sampling config types |
| src/lib/observability/spanProcessor.ts | New span processing pipeline (redaction/truncation/etc.) |
| src/lib/observability/sampling/samplers.ts | New sampler implementations + factory |
| src/lib/observability/sampling/index.ts | Barrel export for sampling module |
| src/lib/observability/otelBridge.ts | New OTel↔NeuroLink context propagation bridge |
| src/lib/observability/index.ts | Main observability exports barrel |
| src/lib/observability/exporters/sentryExporter.ts | New Sentry exporter |
| src/lib/observability/exporters/posthogExporter.ts | New PostHog exporter |
| src/lib/observability/exporters/otelExporter.ts | New OTLP exporter (batch) |
| src/lib/observability/exporters/langsmithExporter.ts | New LangSmith exporter |
| src/lib/observability/exporters/langfuseExporter.ts | New Langfuse exporter |
| src/lib/observability/exporters/laminarExporter.ts | New Laminar exporter |
| src/lib/observability/exporters/index.ts | Barrel export for exporters |
| src/lib/observability/exporters/datadogExporter.ts | New Datadog exporter |
| src/lib/observability/exporters/braintrustExporter.ts | New Braintrust exporter |
| src/lib/observability/exporters/baseExporter.ts | New BaseExporter + NoOp exporter |
| src/lib/observability/exporters/arizeExporter.ts | New Arize exporter |
| src/lib/observability/FEATURE-STATUS.md | New internal status doc for observability subsystem |
| src/lib/memory/memoryRetrievalTools.ts | Adds MEMORY span to retrieval tool |
| src/lib/mcp/mcpClientFactory.ts | Replaces OTel tracer usage with metrics span recording for MCP connect |
| src/lib/mcp/httpRetryHandler.ts | Adds MCP retry span with attempts attribute |
| src/lib/mcp/httpRateLimiter.ts | Adds MCP rateLimit span + waited attribute |
| src/lib/index.ts | Re-exports observability utilities/types and reorders workflow exports |
| src/lib/features/ppt/slideTypeInference.ts | Broadens title keyword matching patterns |
| src/lib/features/ppt/slideRenderers.ts | Hardens social contact rendering type checks |
| src/lib/features/ppt/slideGenerator.ts | Adds PPT span on slide generation |
| src/lib/features/ppt/presentationOrchestrator.ts | Adds PPT orchestration span; records to aggregator + neurolink |
| src/lib/evaluation/scoring.ts | Adds EVALUATION span on scoring mapping |
| src/lib/evaluation/ragasEvaluator.ts | Adds EVALUATION span around RAGAS evaluation |
| src/lib/core/modules/GenerationHandler.ts | Adjusts structured-output enabling logic; import ordering |
| src/lib/core/conversationMemoryManager.ts | Implements no-op close() for in-memory manager |
| src/lib/core/conversationMemoryInitializer.ts | Prefers redis when redisConfig provided (over env STORAGE_TYPE) |
| src/lib/core/baseProvider.ts | Adds metrics spans + OTel span context wiring for generate/stream |
| src/lib/context/contextCompactor.ts | Adds compaction spans + LLM summarize timeout + error attribution |
| src/lib/context/budgetChecker.ts | Adds compaction budget-check span + result attributes |
| src/lib/constants/enums.ts | Adds Gemini 3.1 model constants (released March 2026) |
| src/lib/constants/contextWindows.ts | Adds context windows for Gemini 3.1 models |
| src/lib/adapters/video/vertexVideoHandler.ts | Defers credential validation + supports global host routing |
| src/lib/adapters/tts/googleTTSHandler.ts | Adds TTS spans for listVoices + synthesize |
| src/cli/utils/formatters.ts | New shared CLI formatting helpers |
| src/cli/parser.ts | Swaps docs/auth/workflow commands for observability/telemetry commands |
| src/cli/factories/commandFactory.ts | Fixes static/instance method usage; adds TTS options plumbing |
| package.json | Updates OTEL SDK dependency versions and adds overrides |
| memory-bank/research/CONTINUOUS-TEST-SUITE-EXECUTION-RESULTS.md | Adds test-suite results doc |
| landing/src/routes/api/og/templates.ts | Removes HTML escaping from OG templates |
| landing/src/routes/api/og/fonts.ts | Switches Inter font URLs and removes fetch timeout/ok checks |
| landing/src/routes/api/og/+server.ts | Simplifies wasm init; adjusts response construction |
| landing/src/routes/+layout.svelte | Updates SEO metadata / FAQ copy |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
landing/src/routes/api/og/+server.ts:1
wasmInitializedis set totrueeven ifinitWasm(...)fails (network error, non-200, etc.) because the catch block swallows all errors. That can leave the process in a permanently broken state where subsequent requests skip initialization but rendering fails. Recommendation (mandatory): only setwasmInitialized = trueafter successful initialization, or restrict the catch to the specific "already initialized" error case and rethrow for real failures.
import type { RequestHandler } from "./$types";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| <div style="display:flex;font-size:18px;color:${COLORS.blue};font-weight:600;text-transform:uppercase;letter-spacing:2px;">${section}</div> | ||
| </div> | ||
| <div style="display:flex;font-size:52px;font-weight:700;color:${COLORS.text};line-height:1.15;">${escapeHtml(title)}</div> | ||
| <div style="display:flex;font-size:52px;font-weight:700;color:${COLORS.text};line-height:1.15;">${title}</div> |
| <div style="display:flex;font-size:48px;font-weight:700;color:${COLORS.text};line-height:1.15;margin-bottom:16px;">${title}</div> | ||
| <div style="display:flex;font-size:22px;color:${COLORS.muted};line-height:1.4;">${subtitle}</div> |
| SpanSerializer.updateAttributes(span, { | ||
| "compaction.stage3.error": err.message, | ||
| "compaction.stage3.errorName": err.name, | ||
| "compaction.stage3.tokensBefore": stageTokensBefore, | ||
| "compaction.stage3_failed": true, | ||
| }); |
| ): void; | ||
|
|
||
| /** Trace context propagated from NeuroLink SDK for parent-child span hierarchy */ | ||
| _traceContext?: { traceId: string; parentSpanId: string } | null; |
| "@opentelemetry/exporter-trace-otlp-http": "^0.202.0", | ||
| "@opentelemetry/resources": "^2.1.0", | ||
| "@opentelemetry/sdk-node": "^0.56.0", | ||
| "@opentelemetry/sdk-node": "^0.202.0", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/lib/adapters/video/vertexVideoHandler.ts (2)
100-113:⚠️ Potential issue | 🟡 Minor
isVertexVideoConfigured()may returnfalseeven when ADC credentials are available.The function now explicitly allows ADC (Application Default Credentials) to be resolved at runtime by
GoogleAuth, but returnsfalsewhen only ADC exists. This creates a semantic gap: callers using this function as a gate (e.g.,generateTransitionWithVertexat line 773) will reject requests that would actually succeed.Consider either:
- Renaming to
hasExplicitVertexCredentials()to clarify semantics, or- Attempting a quick ADC probe (though this adds latency), or
- Documenting the caveat in the JSDoc that
falsedoesn't mean video generation will fail.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/adapters/video/vertexVideoHandler.ts` around lines 100 - 113, isVertexVideoConfigured() currently returns false when only ADC (Application Default Credentials) are available which causes callers like generateTransitionWithVertex to incorrectly gate successful runtime credential resolution; either rename isVertexVideoConfigured to hasExplicitVertexCredentials (update all callers including generateTransitionWithVertex) to make semantics explicit, or change the implementation to probe ADC (e.g., quick GoogleAuth.getClient()/getAccessToken check) and return true on success, or add JSDoc on isVertexVideoConfigured explaining that a false value only means no explicit env creds are present and does not guarantee failure at runtime—choose one approach and apply consistently to the function and its callers.
809-809:⚠️ Potential issue | 🟠 MajorInconsistent global endpoint handling in
generateTransitionWithVertex().This function still uses the hardcoded regional pattern
${location}-aiplatform.googleapis.com, whilegenerateVideoWithVertex()was updated to handlelocation === "global"with the non-prefixed host. If a caller passesregion="global", this will construct an invalid endpoint (global-aiplatform.googleapis.com).🐛 Proposed fix to add global endpoint support
// Use Veo 3.1 Fast for transitions (faster with minimal quality difference) - const endpoint = `https://${location}-aiplatform.googleapis.com/v1/projects/${project}/locations/${location}/publishers/google/models/${VEO_FAST_MODEL}:predictLongRunning`; + const apiHost = + location === "global" + ? "aiplatform.googleapis.com" + : `${location}-aiplatform.googleapis.com`; + const endpoint = `https://${apiHost}/v1/projects/${project}/locations/${location}/publishers/google/models/${VEO_FAST_MODEL}:predictLongRunning`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/adapters/video/vertexVideoHandler.ts` at line 809, generateTransitionWithVertex builds the endpoint with a hardcoded regional host pattern which breaks for location === "global"; update the endpoint construction in generateTransitionWithVertex so it mirrors generateVideoWithVertex's logic (use "aiplatform.googleapis.com" when location === "global", otherwise use `${location}-aiplatform.googleapis.com`) when forming the URL that includes VEO_FAST_MODEL and the :predictLongRunning path; modify the endpoint variable creation to use a conditional/utility that selects the correct host and then interpolate the project, location and model into the final URL.landing/src/routes/api/og/templates.ts (1)
51-62:⚠️ Potential issue | 🟠 MajorCritical: Restore HTML escaping for user-provided input.
User-provided query parameters (
section,title) are injected directly into HTML markup without sanitization. Per the relevant code in+server.ts(lines 22-27), these values come straight from URL query params. While the output is rendered to PNG, injecting unsanitized input can cause template breakage, SVG-based attacks, or unexpected behavior in satori's parsing.The same issue affects
sdkTemplate(lines 64-76) andexamplesTemplate(lines 78-93).🛡️ Proposed fix: Add escapeHtml helper and apply to all user inputs
+function escapeHtml(str: string): string { + return str + .replace(/&/g, "&") + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + function docsTemplate(title: string, section: string): string { return wrap(` ${logoBar()} <div style="display:flex;flex-direction:column;flex:1;justify-content:center;"> <div style="display:flex;align-items:center;gap:8px;margin-bottom:16px;"> - <div style="display:flex;font-size:18px;color:${COLORS.blue};font-weight:600;text-transform:uppercase;letter-spacing:2px;">${section}</div> + <div style="display:flex;font-size:18px;color:${COLORS.blue};font-weight:600;text-transform:uppercase;letter-spacing:2px;">${escapeHtml(section)}</div> </div> - <div style="display:flex;font-size:52px;font-weight:700;color:${COLORS.text};line-height:1.15;">${title}</div> + <div style="display:flex;font-size:52px;font-weight:700;color:${COLORS.text};line-height:1.15;">${escapeHtml(title)}</div> </div> ${footerBar()} `); }Apply the same pattern to
sdkTemplate(method,subtitle) andexamplesTemplate(title,subtitle).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@landing/src/routes/api/og/templates.ts` around lines 51 - 62, The templates inject raw user inputs into HTML; add an escapeHtml helper and use it wherever query-derived strings are interpolated: wrap section and title in docsTemplate, method and subtitle in sdkTemplate, and title and subtitle in examplesTemplate (e.g., replace ${section}, ${title}, ${method}, ${subtitle} with escaped variants). Ensure escapeHtml properly encodes <, >, &, ", ' and backticks before interpolation so wrap/logoBar/footerBar usage remains unchanged and satori receives sanitized strings.src/lib/adapters/tts/googleTTSHandler.ts (1)
97-128:⚠️ Potential issue | 🟠 MajorSet
durationMsbefore recording these TTS spans.
getVoices()andsynthesize()both callendSpan()without ever populatingspan.durationMs. Insynthesize(),latencyis computed separately, so the returned metadata can disagree with the stored span. These spans will report zero/undefined latency and skew TTS metrics/exporters.Also applies to: 177-187, 214-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/adapters/tts/googleTTSHandler.ts` around lines 97 - 128, The created TTS spans (SpanSerializer.createSpan with SpanType.TTS and name "tts.google.listVoices") never have span.durationMs set before calling SpanSerializer.endSpan, causing zero/incorrect latency; fix getVoices() and synthesize() by computing durationMs just prior to ending/recording the span (e.g., capture a start timestamp when creating the span or use span.startTime if present, compute durationMs = Date.now() - start, assign it to span.durationMs) and then call SpanSerializer.endSpan(span, SpanStatus.OK/ERROR) and getMetricsAggregator().recordSpan(endedSpan); apply the same change for the other span usages noted (lines near synthesize() and other TTS spans).src/lib/core/baseProvider.ts (1)
1738-1789:⚠️ Potential issue | 🔴 CriticalDo not dereference arbitrary URL/path strings from
input.images[0]on the server.This turns caller input into server-side fetches and filesystem reads with no private-network, host, or path guard. In hosted deployments that accept user-controlled image references, that is an SSRF/filesystem-read primitive and can forward internal data to Vertex. Restrict this to trusted sources or add explicit URL and filesystem validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/core/baseProvider.ts` around lines 1738 - 1789, The code currently dereferences imageInput (string) into server-side fetches and filesystem reads (see imageInput handling, executeWithTimeout, logger.debug, VideoError), which enables SSRF/filesystem-read risks; add input validation and trust checks before any network or disk access: implement and call a validator (e.g., isTrustedImageSource/validateImageInput) that (1) if URL: parse and reject non-https, reject hostnames resolving to private/loopback/reserved IPs, enforce an allowlist or require a configured trusted-hosts flag, and disallow redirects to untrusted hosts; (2) if file path: require paths only under a configured allowed base directory using path.resolve and ensure no path traversal, and require an explicit config flag to allow local reads; finally, only proceed to call fetch or fs.readFile inside the existing branches after the validator approves, and throw a VideoError with clear context if validation fails.
🟡 Minor comments (14)
landing/src/routes/+layout.svelte-231-231 (1)
231-231:⚠️ Potential issue | 🟡 MinorClarify pruning behavior in the context-compaction FAQ text.
At Line 231, the phrase “replaces old tool results with placeholders” reads as unconditional. It should explicitly mention preserving the recent-token safety window to avoid a misleading behavior claim.
✏️ Suggested text tweak
- text: "NeuroLink includes a 4-stage context compaction pipeline that runs automatically: (1) tool output pruning — replaces old tool results with placeholders, (2) file read deduplication — keeps only the latest read of each file, (3) LLM summarization — structured 9-section summaries with iterative merging, and (4) sliding window truncation. The BudgetChecker triggers auto-compaction when context usage exceeds 80%.", + text: "NeuroLink includes a 4-stage context compaction pipeline that runs automatically: (1) tool output pruning — replaces older tool results with placeholders while preserving the recent ~40K tokens, (2) file read deduplication — keeps only the latest read of each file, (3) LLM summarization — structured 9-section summaries with iterative merging, and (4) sliding window truncation. The BudgetChecker triggers auto-compaction when context usage exceeds 80%.",Based on learnings: Tool output pruning should replace old results with placeholders while protecting recent 40K tokens.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@landing/src/routes/`+layout.svelte at line 231, Update the FAQ string that describes the 4-stage context compaction pipeline (the text variable containing "NeuroLink includes a 4-stage context compaction pipeline...") so the pruning description explicitly states it preserves a recent-token safety window; change "replaces old tool results with placeholders" to something like "replaces older tool results with placeholders while preserving a recent-token safety window (≈40K tokens)" to avoid implying unconditional deletion and to match the documented behavior.landing/src/routes/api/og/fonts.ts-33-38 (1)
33-38:⚠️ Potential issue | 🟡 MinorMissing error handling for failed font fetches.
The simplified fetch removes both timeout protection and HTTP status validation. If the font CDN returns a non-2xx response (404, 500, etc.) or the request hangs, the code will either cache invalid data or block indefinitely.
Consider adding basic error handling:
🛡️ Proposed fix
const buffers = await Promise.all( INTER_FONTS.map(async (font) => { const res = await fetch(font.url); + if (!res.ok) { + throw new Error(`Failed to fetch font: ${res.status}`); + } return res.arrayBuffer(); }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@landing/src/routes/api/og/fonts.ts` around lines 33 - 38, The font fetch logic using INTER_FONTS.map currently lacks error handling and timeout; update the code that builds buffers (the Promise.all over INTER_FONTS.map) to (1) use an AbortController with a short timeout for each fetch to avoid hanging requests, (2) check response.ok after each fetch and throw a descriptive error if the HTTP status is non-2xx, and (3) propagate or handle fetch failures so Promise.all doesn't cache/return invalid ArrayBuffers (e.g., reject the overall operation and log the font name/url and status). Reference INTER_FONTS, the map callback that calls fetch(font.url), and the buffers Promise.all aggregation when making these changes.memory-bank/research/CONTINUOUS-TEST-SUITE-FIX-PLAN.md-208-208 (1)
208-208:⚠️ Potential issue | 🟡 MinorEscape
||inside markdown table cells to prevent column corruption.Literal
||in table content is parsed as extra column separators in many renderers/linters. Escape them (\|\|) or wrap the cell with HTML code formatting.Suggested doc diff
-| 5.19 | Observability Spans | Synthetic fallback masks real failures | Remove the OR condition (`realWorkflowPassed || syntheticPassed`). If the real workflow can't run, the test should SKIP, not silently fall back to synthetic. | +| 5.19 | Observability Spans | Synthetic fallback masks real failures | Remove the OR condition (`realWorkflowPassed \|\| syntheticPassed`). If the real workflow can't run, the test should SKIP, not silently fall back to synthetic. | -| 8.6 | RAG with Generate | `|| generateResult` makes any truthy object pass | Remove the `||` fallback. Assert `generateResult.content.length > 50`. Assert content is relevant to the query (contains at least one keyword from the RAG documents). | +| 8.6 | RAG with Generate | `\|\| generateResult` makes any truthy object pass | Remove the `\|\|` fallback. Assert `generateResult.content.length > 50`. Assert content is relevant to the query (contains at least one keyword from the RAG documents). |Also applies to: 307-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@memory-bank/research/CONTINUOUS-TEST-SUITE-FIX-PLAN.md` at line 208, The table cell containing the literal boolean expression "realWorkflowPassed || syntheticPassed" is breaking markdown table parsing; update the cell in CONTINUOUS-TEST-SUITE-FIX-PLAN.md (the row labeled "5.19 Observability Spans") to escape the pipe characters or render as code (e.g., replace "realWorkflowPassed || syntheticPassed" with "\|\|"/escaped form or wrap it in inline code) and make the same change for the other occurrence referenced in the comment so the table columns remain intact.memory-bank/research/CONTINUOUS-TEST-SUITE-EXECUTION-RESULTS.md-56-58 (1)
56-58:⚠️ Potential issue | 🟡 MinorFix contradictory skip summary.
Line 58 says all skips are due to unimplemented features, but Line 56 classifies provider skips as rate-limit fallout. Please split skips into at least two buckets (feature-missing vs environment/external) to keep triage accurate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@memory-bank/research/CONTINUOUS-TEST-SUITE-EXECUTION-RESULTS.md` around lines 56 - 58, The skip-summary is contradictory: the table header row ("Providers | OpenRouter Tool Use (x2) | Rate limited after streaming failure | Rate limit") implies environment/external rate-limit skips while the sentence "All skips are features that genuinely don't exist yet in the SDK." incorrectly lumps them as feature-missing; update the document to split skips into two clear buckets—"Feature missing (SDK)" and "Environment/External (rate-limits, streaming failures)"—move or reclassify the OpenRouter/Rate limited entries into the Environment/External bucket, and replace the single-line assertion with two concise summary lines that enumerate counts/examples for each bucket so triage is accurate.src/lib/features/ppt/slideTypeInference.ts-88-91 (1)
88-91:⚠️ Potential issue | 🟡 MinorUse a more robust pattern for matching "vs" or "vs." in titles
The regex
/\bvs\.?\b/ion line 90 fails to match titles containing "vs." (e.g., "A vs. B") because the trailing word boundary\bcannot occur after the period when followed by a word character. This causes comparison slides with "vs." notation to be skipped.Use
/(?<!\w)vs\.?(?!\w)/iinstead, which uses negative lookbehind and lookahead to safely match both "vs" and "vs." without relying on word boundaries around punctuation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/features/ppt/slideTypeInference.ts` around lines 88 - 91, The regex `/\bvs\.?\b/i` in slideTypeInference.ts fails to match titles like "A vs. B" because the trailing `\b` can't occur after a period when followed by a word character; replace that pattern with a robust boundary-free variant using lookarounds (e.g., use a pattern that matches `vs` or `vs.` with negative lookbehind/lookahead) so the comparison detection regex (the `vs` entry in the array of patterns used by the slide type inference logic) correctly matches both "vs" and "vs." cases.src/lib/observability/otelBridge.ts-100-100 (1)
100-100:⚠️ Potential issue | 🟡 MinorUnsafe type assertion for
recordException.The
error as Errorcast will pass non-Error values torecordException, which expects anErroror string. Since you already checkerror instanceof Errorabove, use that pattern here.🛡️ Proposed fix
- otelSpan.recordException(error as Error); + if (error instanceof Error) { + otelSpan.recordException(error); + } else { + otelSpan.recordException(new Error(String(error))); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/otelBridge.ts` at line 100, Replace the unsafe assertion "error as Error" when calling otelSpan.recordException: use the existing type check (error instanceof Error) to pass the Error directly to otelSpan.recordException, and otherwise pass a string representation (e.g., String(error) or error?.toString()) so recordException always receives an Error or a string; locate the call to otelSpan.recordException and the surrounding error handling that does the instanceof check to implement this change.src/lib/workflow/core/ensembleExecutor.ts-123-130 (1)
123-130:⚠️ Potential issue | 🟡 MinorMark unmet
minResponsesas an error span.When
successCount < minResponses, this function adds anINSUFFICIENT_RESPONSESerror but still records the span as OK if one model succeeded. Also, because finalization happens only on the return path, unexpected throws won't emit any span at all.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/workflow/core/ensembleExecutor.ts` around lines 123 - 130, The span finalization currently sets spanStatus based only on successCount > 0 and can miss marking INSUFFICIENT_RESPONSES or emitting a span on throws; update the finalization in ensembleExecutor.ts so that before calling SpanSerializer.endSpan you check if successCount < minResponses and, if so, set spanStatus = SpanStatus.ERROR and add an error/attribute indicating "INSUFFICIENT_RESPONSES" to span; also move the SpanSerializer.endSpan + getMetricsAggregator().recordSpan calls into a finally block that always runs (so spans are emitted on exceptions) and include any caught error details when ending the span to ensure accurate error reporting.src/lib/utils/ttsProcessor.ts-332-347 (1)
332-347:⚠️ Potential issue | 🟡 MinorUse UTF-8 byte length for provider limits.
maxTextLengthis documented as bytes, but these checks use JavaScript string length. Emoji/CJK text can pass validation here and still exceed the provider limit, and the recordedtextLengthwill be wrong.Also applies to: 416-420
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/ttsProcessor.ts` around lines 332 - 347, The check uses JavaScript string length but maxTextLength is in UTF-8 bytes; compute const textByteLength = Buffer.byteLength(trimmedText, 'utf8') and compare textByteLength > maxTextLength instead of trimmedText.length, update the thrown TTSError message and context (textLength: textByteLength) and any logger output to report byte length, and apply the same replacement for the other identical check that references maxTextLength/handler.maxTextLength and trimmedText in this module.src/lib/observability/exporters/posthogExporter.ts-65-76 (1)
65-76:⚠️ Potential issue | 🟡 Minor
getHeaders()method is not used for export requests.The
getHeaders()method (lines 65-76) builds authorization headers withpersonalApiKey, butexportSpanandexportBatchdon't use it - they only setContent-Type. This means thepersonalApiKeyconfig option has no effect on export operations, which may not match user expectations.Either use
getHeaders()in export methods or document thatpersonalApiKeyis only used for management operations.Also applies to: 84-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/posthogExporter.ts` around lines 65 - 76, The export methods exportSpan and exportBatch are not using the getHeaders() helper so the personalApiKey Authorization header is never sent; update both exportSpan and exportBatch to call this.getHeaders(), merge/override any existing headers (ensuring "Content-Type": "application/json" remains), and use that combined headers object for the fetch/HTTP request so personalApiKey is honored for export operations.src/lib/observability/exporters/posthogExporter.ts-44-50 (1)
44-50:⚠️ Potential issue | 🟡 MinorTreating HTTP 401 as acceptable during initialization is misleading.
The code treats 401 (Unauthorized) as acceptable because "401 is expected with project API key", but this suggests the API key validation isn't actually working. If the project API key can't authenticate against
/api/projects/, this check provides no value. Consider either:
- Using an endpoint that works with project API keys
- Removing this validation if it can't reliably verify credentials
- Using the
/capture/endpoint with a test event🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/posthogExporter.ts` around lines 44 - 50, The current init check treats response.status === 401 as acceptable (response.ok and response.status checks around logger.warn/response.statusText), which is misleading; update the verification in posthogExporter initialization to either remove the 401 special-case or switch to a test /capture request: call the PostHog /capture endpoint with a minimal test event using the project API key, treat any non-2xx (including 401) as a failure, and replace the existing logger.warn(...) branch (which references response.statusText) with an error or warning that includes the real response details; ensure you update the code paths that reference response.ok, response.status, logger.warn, and response.statusText accordingly.src/lib/observability/exporters/otelExporter.ts-153-155 (1)
153-155:⚠️ Potential issue | 🟡 MinorgRPC protocol option is declared but not implemented.
The
getExportUrlmethod handles thegrpcprotocol case but just returns the raw endpoint. The comment indicates gRPC support would require@grpc/grpc-js, but there's no actual gRPC implementation. This could mislead users who configureprotocol: "grpc"expecting it to work.Consider either:
- Removing
grpcfrom the supported protocols until implemented- Throwing an error when gRPC is selected
- Adding a warning log when gRPC is configured
🛡️ Suggested fix to warn on unsupported protocol
case "grpc": - // For gRPC, this would use `@grpc/grpc-js` + // gRPC not yet implemented - warn and fall back to HTTP + logger.warn("[OtelExporter] gRPC protocol not implemented, falling back to HTTP"); return this.endpoint;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/otelExporter.ts` around lines 153 - 155, The getExportUrl method currently returns the raw endpoint for the case "grpc" (case "grpc" in getExportUrl) but doesn't implement gRPC; update getExportUrl to handle this intentionally by either removing/supporting gRPC or making it explicit: replace the current case "grpc" behavior with a clear error or warning and guidance—e.g., throw an Error("gRPC protocol not supported") or call logger.warn("gRPC protocol configured but not implemented")—so callers of getExportUrl (and any code referencing this.endpoint) aren't misled; ensure the change references the case "grpc" in getExportUrl and update any upstream callers to expect the thrown error/warning behavior.src/cli/commands/observability.ts-74-80 (1)
74-80:⚠️ Potential issue | 🟡 Minor
--format tableis advertised, but none of these handlers render a table.Every non-JSON path falls back to the same text formatter, so
tableis a broken CLI contract today. Either implement a real table renderer or removetablefrom the allowed choices until it exists.Also applies to: 99-101, 185-191, 214-216, 364-370, 391-393, 477-483, 512-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/observability.ts` around lines 74 - 80, The CLI advertises a "table" output but the option("format", ...) in observability.ts currently lists "table" while all command handlers still route non-JSON output to the same text formatter; either remove "table" from the choices array in option("format") to avoid a broken CLI contract, or implement a real table renderer and wire it into every place the format is handled (where the code selects between "json" and text) by adding a renderTable function and switching handlers to call renderTable when format === "table" instead of falling back to text. Ensure you update option("format") and every command handler that formats output to consistently support the new "table" path.src/lib/observability/exporters/langfuseExporter.ts-89-99 (1)
89-99:⚠️ Potential issue | 🟡 MinorBatch error reporting can point at the wrong span.
After
filter(),ino longer lines up with the originalspansarray, so a later failure can be reported against an earlier span ID. This path also throws away the concrete error text fromexportSpan()and replaces it with a generic"Export failed".Suggested fix
- errors: results - .filter( - (r) => - r.status === "rejected" || - (r.status === "fulfilled" && !r.value.success), - ) - .map((r, i) => ({ - spanId: spans[i].spanId, - error: r.status === "rejected" ? String(r.reason) : "Export failed", - retryable: true, - })), + errors: results.flatMap((entry, index) => { + if (entry.status === "rejected") { + return [ + { + spanId: spans[index].spanId, + error: String(entry.reason), + retryable: true, + }, + ]; + } + return entry.value.success ? [] : (entry.value.errors ?? []); + }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/langfuseExporter.ts` around lines 89 - 99, The failure list is misaligned because you filter results and then use the filtered index `i` to pick `spans[i]`, which no longer matches the original span positions and also loses concrete error text; change the logic to first map over `results` with their original index (e.g., results.map((r, idx) => ({ r, idx }))) and then filter those entries, or use results.reduce to build errors while carrying the original `idx`, and when creating each error object use spans[idx].spanId and preserve the real error text from the fulfilled value (e.g., r.value?.error ?? "Export failed") and any retryable flag from r.value (e.g., r.value?.retryable ?? true) so errors point to the correct span and include concrete messages.src/lib/neurolink.ts-6416-6424 (1)
6416-6424:⚠️ Potential issue | 🟡 MinorUse resolved provider name for failed-stream span attribution (avoid
provider: "auto").Line 6418 can record
"auto"as provider in failed spans, which skews telemetry and root-cause triage when fallback kicks in.🔧 Suggested fix
+ const resolvedProviderForSpan = + options.provider && options.provider !== "auto" + ? options.provider + : await getBestProvider(options.provider); + // Record a failed-provider span for the primary provider that threw try { - const failedProvider = options.provider || "unknown"; + const failedProvider = resolvedProviderForSpan; const traceCtx = this._metricsTraceContext; let failedSpan = SpanSerializer.createGenerationSpan({ provider: failedProvider, @@ - const providerName = await getBestProvider(options.provider); + const providerName = resolvedProviderForSpan;Also applies to: 6439-6443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 6416 - 6424, The failed-span code uses options.provider which can be the placeholder "auto"; change it to use the resolved provider name used for the actual request (e.g., the variable or getter that holds the concrete provider used—resolveProvider / this._resolvedProvider / the same value passed to the request) before calling SpanSerializer.createGenerationSpan so provider is never "auto"; update both the failed-stream span creation (where failedProvider is set) and the other analogous span block later (the similar createGenerationSpan call around the second failure) to reference the resolved provider variable instead of options.provider or literal "auto".
🧹 Nitpick comments (15)
src/lib/adapters/video/vertexVideoHandler.ts (1)
935-941: Stale line number reference in comment.The comment references "line 374" but the
predictLongRunningendpoint construction is now around lines 401-405. Consider updating or removing the specific line reference to avoid confusion during future maintenance.📝 Suggested fix
- // Global endpoint uses aiplatform.googleapis.com (no region prefix), - // same pattern as the predictLongRunning endpoint at line 374 + // Global endpoint uses aiplatform.googleapis.com (no region prefix), + // same pattern as the predictLongRunning endpoint in generateVideoWithVertex()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/adapters/video/vertexVideoHandler.ts` around lines 935 - 941, The comment above pollHost/pollEndpoint contains a stale line reference to "line 374" for the predictLongRunning endpoint; update the comment to either remove the specific line number or point to the correct location where predictLongRunning is constructed (e.g., reference the predictLongRunning construction instead of a line number), so change the comment near pollHost/pollEndpoint (symbols: pollHost, pollEndpoint, location, modelOrEndpoint) to avoid hardcoded line numbers and improve maintainability.landing/src/routes/+layout.svelte (1)
127-183: Consider reducing duplicated capability counts in FAQ copy.The same numeric claims (provider count/chunking/vector store/file types) are hard-coded across multiple strings, which can drift over time. Consider building these strings from shared constants to keep marketing and schema content in sync.
memory-bank/research/CONTINUOUS-TEST-SUITE-FIX-PLAN.md (1)
48-49: Explicitly require case-normalized provider error matching in helper spec.Please add that
isExpectedProviderError()lowercases the message before matching to keep skip/error classification stable across provider casing variations.Based on learnings: continuous suites should use case-insensitive provider error detection via
toLowerCase()and represent skipped scenarios withnull.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@memory-bank/research/CONTINUOUS-TEST-SUITE-FIX-PLAN.md` around lines 48 - 49, Update the helper isExpectedProviderError() to perform case-insensitive matching by lowercasing the provider error message before comparing (e.g., call message.toLowerCase()) so skips/errors are stable across provider casing; ensure the function still filters out the removed patterns ("not found", "unknown error", "bad request") and that validateResponseContent() and any callers treat skipped scenarios as null (represent skipped responses by returning null) to keep the continuous test-suite behavior consistent.src/lib/observability/otelBridge.ts (2)
81-82: UseSpanStatusCodeenum instead of magic numbers.Using numeric literals
1and2for span status codes reduces readability and type safety. Import and useSpanStatusCodefrom@opentelemetry/api.♻️ Proposed fix
import { context, propagation, type SpanContext, + SpanStatusCode, trace, } from "@opentelemetry/api";Then update the status code usages:
- otelSpan.setStatus({ code: 1 }); // OK + otelSpan.setStatus({ code: SpanStatusCode.OK });- otelSpan.setStatus({ - code: 2, // ERROR - message: error instanceof Error ? error.message : String(error), - }); + otelSpan.setStatus({ + code: SpanStatusCode.ERROR, + message: error instanceof Error ? error.message : String(error), + });- const otelStatusCode = span.status === SpanStatus.ERROR ? 2 : 1; + const otelStatusCode = span.status === SpanStatus.ERROR ? SpanStatusCode.ERROR : SpanStatusCode.OK;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/otelBridge.ts` around lines 81 - 82, Replace magic numeric status codes with the SpanStatusCode enum: import { SpanStatusCode } from '@opentelemetry/api', then update calls like otelSpan.setStatus({ code: 1 }) and any other occurrences using 1/2 to use SpanStatusCode.OK and SpanStatusCode.ERROR respectively (relating to SpanSerializer.endSpan, neuroLinkSpan, and otelSpan.setStatus usage).
115-143: Consider setting parent span context for trace hierarchy.
exportToOtelcreates a new span but doesn't link it to the original trace hierarchy. Thespan.traceIdandspan.parentSpanIdare available inSpanDatabut aren't used, which could result in orphaned spans in the trace view.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/otelBridge.ts` around lines 115 - 143, exportToOtel currently starts a new span without linking it to the original trace, so use span.traceId and span.parentSpanId to preserve the trace hierarchy: before calling tracer.startSpan in exportToOtel, construct a parent span context from span.traceId and span.parentSpanId and provide it as the parent (or add a link to that SpanContext) in the tracer.startSpan options; update the call site (the exportToOtel function and its use of tracer.startSpan) to pass the parent context or link so spans are not orphaned.src/lib/server/abstract/baseServerAdapter.ts (1)
666-681: Differentiate forced-close shutdowns from clean drains.This branch only runs after a
ShutdownTimeoutErrororDrainTimeoutError, but the span is still emitted asOK. That makes timeout-driven force closes invisible in telemetry. At minimum, attach aserver.forcedClose/timeout attribute here; ideally don't classify it the same as a normal graceful shutdown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/abstract/baseServerAdapter.ts` around lines 666 - 681, The shutdown branch for forceClose (checked via forceClose and error instanceof ShutdownTimeoutError || error instanceof DrainTimeoutError) currently ends the span with SpanStatus.OK; change this so the span reflects a forced/timeout shutdown: set a server.forcedClose (or server.shutdown.timeout) attribute on shutdownSpan (or its attributes map) to true and include the error message, then end the span with a non-OK status (e.g., SpanStatus.ERROR or a distinct timeout status) before calling getMetricsAggregator().recordSpan; keep the existing calls to logger.warn, this.forceCloseConnections, and this.closeServer but ensure shutdownSpan and SpanSerializer.endSpan reflect the forced-close/timeout state rather than OK.src/lib/providers/googleVertex.ts (2)
1716-1730: Duplicated system preamble injection logic between stream and generate paths.The system instruction handling for the global endpoint (lines 1716-1730 and 1996-2008) is identical in both
executeNativeGemini3StreamandexecuteNativeGemini3Generate. Consider extracting this into a shared helper method to reduce duplication and ensure consistent behavior.♻️ Suggested helper extraction
private prependSystemPreamble( contents: Array<{ role: string; parts: unknown[] }>, systemPreamble: string | undefined, ): Array<{ role: string; parts: unknown[] }> { if (!systemPreamble) { return contents; } return [ { role: "user", parts: [{ text: `[System Instructions]\n${systemPreamble}` }], }, { role: "model", parts: [{ text: "OK" }], }, ...contents, ]; }Also applies to: 1996-2008
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/providers/googleVertex.ts` around lines 1716 - 1730, Both executeNativeGemini3Stream and executeNativeGemini3Generate duplicate the same system preamble injection logic; extract that logic into a shared private helper (e.g., prependSystemPreamble) that accepts the current contents array and an optional systemPreamble string and returns the new contents with the two preamble entries prepended when systemPreamble is present; then replace the duplicated blocks in executeNativeGemini3Stream and executeNativeGemini3Generate to call this helper so both paths share the same behavior and avoid divergence.
1552-1554: Consider accepting the fullChatMessagetype instead of a narrower inline type.The parameter type
Array<{ role: string; content: string }>is narrower than the actualChatMessagetype that callers pass (which includesid,timestamp,tool, etc.). While TypeScript's structural typing allows this, using the importedChatMessagetype would be more explicit and self-documenting.Additionally, silently dropping
tool_callandtool_resultmessages (lines 1566-1568) may lose important context in multi-turn tool-using conversations. Consider whether these should be converted to a text representation instead of being completely omitted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/providers/googleVertex.ts` around lines 1552 - 1554, Change the parameter type of prependConversationHistory to accept the full ChatMessage[] (import and use ChatMessage) instead of Array<{ role: string; content: string }>, update uses of conversationMessages within prependConversationHistory to rely on ChatMessage properties (id, timestamp, tool, etc.) as needed, and replace the current silent drop of messages with type "tool_call" and "tool_result" (inside prependConversationHistory) by converting them into a readable text representation (e.g., serialize tool name/args or tool result into content) so they are prepended rather than omitted; keep the function name prependConversationHistory unchanged.test/audit/agent-test-tracing-journey.ts (1)
89-128: Consider typing the span parameter properly.The
printSpanfunction acceptsRecord<string, unknown>but could acceptSpanDatafrom the observability types for better type safety. This would provide IDE autocompletion and catch typos in attribute names.♻️ Suggested type improvement
+import type { SpanData } from "../../src/lib/observability/types/spanTypes.js"; + -function printSpan(s: Record<string, unknown>, indent = 0) { +function printSpan(s: SpanData, indent = 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/audit/agent-test-tracing-journey.ts` around lines 89 - 128, The printSpan function currently types its parameter as Record<string, unknown>; change it to use the proper SpanData type from your observability types (e.g., import { SpanData } from the observability package) so the function signature becomes printSpan(s: SpanData, indent = 0); update any places where you access s.attributes to use the typed s.attributes (e.g., const a = s.attributes ?? {}) and adjust any property access to respect the SpanData shape (keep the same runtime checks but rely on TS for autocompletion and to catch typos in attribute names like ai.provider, ai.model, ai.cost.total, stream.chunk_count, tool.name, etc.).src/lib/observability/exporters/braintrustExporter.ts (1)
172-186: Redundant metadata fields inconvertToBraintrustLog.The metadata object spreads
ai.providerandai.modelconditionally, then unconditionally setsproviderandmodelto the same values. This creates duplicate data under different keys. Consider simplifying:♻️ Simplified metadata
metadata: { - // Pick only safe, non-PII attributes for metadata (avoid leaking input/output) - ...(span.attributes["ai.provider"] !== undefined && { - "ai.provider": span.attributes["ai.provider"], - }), - ...(span.attributes["ai.model"] !== undefined && { - "ai.model": span.attributes["ai.model"], - }), - // Explicit fields placed after spread so they always win provider: span.attributes["ai.provider"], model: span.attributes["ai.model"], type: span.type, status: span.status, statusMessage: span.statusMessage, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/braintrustExporter.ts` around lines 172 - 186, In convertToBraintrustLog the metadata object redundantly includes ai.provider/ai.model via conditional spreads and then unconditionally sets provider/model; remove the conditional spreads of "ai.provider" and "ai.model" (the ...(span.attributes["ai.provider"] !== undefined && { "ai.provider": ... }) and corresponding ai.model spread) and keep the explicit provider and model properties (or vice versa if you need the ai.* keys), so that metadata only contains a single set of provider/model keys derived from span.attributes in the convertToBraintrustLog function.src/lib/observability/exporters/otelExporter.ts (1)
186-190: Consider adding request timeouts to fetch calls.The
sendRequestmethod andpingmethod usefetchwithout timeouts. In production, this could cause indefinite hangs if the OTLP endpoint is slow or unresponsive. Consider addingAbortSignal.timeout()for bounded request duration.♻️ Suggested timeout addition
const response = await fetch(endpoint, { method: "POST", headers, body: bodyData, + signal: AbortSignal.timeout(30000), // 30 second timeout });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/exporters/otelExporter.ts` around lines 186 - 190, The fetch calls in sendRequest and ping can hang; update both functions in otelExporter.ts to use an AbortSignal with a bounded timeout (e.g., AbortSignal.timeout or an AbortController with setTimeout) passed via the fetch "signal" option, make the timeout value configurable (eg. OTEL_REQUEST_TIMEOUT_MS or a parameter on the exporter), and handle aborts explicitly (treat AbortError as a timeout in logs/metrics and clean up any timers). Locate the fetch calls inside sendRequest and ping and add the signal wiring and error handling so requests are cancelled after the configured timeout.src/lib/observability/types/exporterTypes.ts (1)
194-198: Don't erase the plugin contract withunknown.Returning
unknownhere forces every plugin consumer to cast before it can call exporter methods, which defeats the strict typing on this extension point. Model this asBaseExporter(or a dedicated exporter interface) instead. As per coding guidelines,**/*.{ts,tsx}: Maintain strict TypeScript type safety across all modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/observability/types/exporterTypes.ts` around lines 194 - 198, The ExporterPlugin type currently returns unknown which forces unsafe casts; update the create signature to return a strongly typed exporter interface (e.g., BaseExporter or a dedicated Exporter interface) instead of unknown. Locate the ExporterPlugin type and change its create(config: ExporterConfig): unknown to create(config: ExporterConfig): BaseExporter (or your chosen exporter interface), and ensure BaseExporter is imported or defined in the same module with the required exporter methods so plugin consumers can call methods without casting. Ensure ExporterConfig remains as-is and update any plugin implementations to conform to the BaseExporter interface.src/cli/commands/telemetry.ts (1)
69-92: Use the shared CLI command factory here.This command group still builds raw
yargsmodules inline instead of going through the repo’s shared command-construction path, so it won’t inherit the same registration and consistency hooks as the other CLI commands.As per coding guidelines, "CLI commands must be created via CommandFactory pattern extending yargs command modules".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/telemetry.ts` around lines 69 - 92, The telemetry command group is built inline instead of using the repo-wide CommandFactory pattern; refactor TelemetryCommandFactory.createTelemetryCommands() to return a command module produced by the shared CommandFactory (or the repo's CommandFactory pattern) rather than constructing raw yargs in-place, and register the existing subcommands (TelemetryCommandFactory.createStatusCommand(), createConfigureCommand(), createListExportersCommand(), createFlushCommand(), createStatsCommand()) through that shared factory so the group inherits the common registration/consistency hooks.src/lib/index.ts (1)
39-39: Move TTS and optional subsystem exports behind subpath entrypoints instead of the root barrel.The root entrypoint currently re-exports
GoogleTTSHandler(line 39) along with observability, workflow, and evaluation modules (lines 137–138, 147–156, 162–167, 528–533, 536–541, 556–587, 951–974) as runtime exports. SinceGoogleTTSHandlerdirectly imports@google-cloud/text-to-speechat module scope, core-only consumers will pay the bundle and startup cost for this optional vendor SDK even if they never use TTS. The observability, workflow, and evaluation modules don't have direct vendor imports, but bundling them at the root still increases the core entrypoint's surface area unnecessarily.Recommend:
- Keep these subsystems behind subpath entrypoints (e.g.,
@juspay/neurolink/workflow,@juspay/neurolink/observability,@juspay/neurolink/evaluation,@juspay/neurolink/tts) so consumers can import them selectively.- This also improves treeshaking for downstream bundlers and keeps the core API surface minimal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/index.ts` at line 39, The root barrel in src/lib/index.ts currently re-exports GoogleTTSHandler (and other subsystem exports) which forces optional vendor imports at module load; remove the direct export of GoogleTTSHandler from the root barrel and instead expose it only via a dedicated subpath entrypoint (e.g., create a tts subpath that exports GoogleTTSHandler from ./adapters/tts/googleTTSHandler.js), and do the same for workflow/observability/evaluation subsystems so the root exports only core symbols; locate references to GoogleTTSHandler in index.ts and delete its export line, then add/create corresponding subpath index files that re-export the subsystem classes (GoogleTTSHandler, workflow handlers, observability, evaluation) so consumers import `@yourpkg/tts`, `@yourpkg/workflow`, etc.src/lib/neurolink.ts (1)
2778-2784: Prefer typed error construction here instead of throwing rawError.Line 2782 introduces a plain
Error; this weakens downstream categorization and structured handling.As per coding guidelines "Use ErrorFactory for creating typed errors".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 2778 - 2784, Replace the plain throw new Error in the input validation block (the check of options.input?.text in this file) with a typed error created via ErrorFactory (e.g., ErrorFactory.create or the project-standard ErrorFactory method) so callers can programmatically identify the failure; construct the error with a clear error code like "InvalidInput" and the same descriptive message ("Input text is required and must be a non-empty string"), and ensure ErrorFactory is imported in this module before use.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9c57edcc-729d-4b8a-909c-52c323cd45c2
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltest/fixtures/sample-screenshot.pngis excluded by!**/*.png
📒 Files selected for processing (96)
docs/plans/2026-03-07-observability-api-wiring.mdlanding/src/routes/+layout.sveltelanding/src/routes/api/og/+server.tslanding/src/routes/api/og/fonts.tslanding/src/routes/api/og/templates.tsmemory-bank/research/CONTINUOUS-TEST-SUITE-ANALYSIS.mdmemory-bank/research/CONTINUOUS-TEST-SUITE-EXECUTION-RESULTS.mdmemory-bank/research/CONTINUOUS-TEST-SUITE-FIX-PLAN.mdpackage.jsonsrc/cli/commands/observability.tssrc/cli/commands/telemetry.tssrc/cli/factories/commandFactory.tssrc/cli/parser.tssrc/cli/utils/formatters.tssrc/lib/adapters/tts/googleTTSHandler.tssrc/lib/adapters/video/vertexVideoHandler.tssrc/lib/constants/contextWindows.tssrc/lib/constants/enums.tssrc/lib/context/budgetChecker.tssrc/lib/context/contextCompactor.tssrc/lib/core/baseProvider.tssrc/lib/core/conversationMemoryInitializer.tssrc/lib/core/conversationMemoryManager.tssrc/lib/core/modules/GenerationHandler.tssrc/lib/evaluation/ragasEvaluator.tssrc/lib/evaluation/scoring.tssrc/lib/features/ppt/presentationOrchestrator.tssrc/lib/features/ppt/slideGenerator.tssrc/lib/features/ppt/slideRenderers.tssrc/lib/features/ppt/slideTypeInference.tssrc/lib/index.tssrc/lib/mcp/httpRateLimiter.tssrc/lib/mcp/httpRetryHandler.tssrc/lib/mcp/mcpClientFactory.tssrc/lib/memory/memoryRetrievalTools.tssrc/lib/neurolink.tssrc/lib/observability/FEATURE-STATUS.mdsrc/lib/observability/exporterRegistry.tssrc/lib/observability/exporters/arizeExporter.tssrc/lib/observability/exporters/baseExporter.tssrc/lib/observability/exporters/braintrustExporter.tssrc/lib/observability/exporters/datadogExporter.tssrc/lib/observability/exporters/index.tssrc/lib/observability/exporters/laminarExporter.tssrc/lib/observability/exporters/langfuseExporter.tssrc/lib/observability/exporters/langsmithExporter.tssrc/lib/observability/exporters/otelExporter.tssrc/lib/observability/exporters/posthogExporter.tssrc/lib/observability/exporters/sentryExporter.tssrc/lib/observability/index.tssrc/lib/observability/metricsAggregator.tssrc/lib/observability/otelBridge.tssrc/lib/observability/retryPolicy.tssrc/lib/observability/sampling/index.tssrc/lib/observability/sampling/samplers.tssrc/lib/observability/spanProcessor.tssrc/lib/observability/tokenTracker.tssrc/lib/observability/types/exporterTypes.tssrc/lib/observability/types/index.tssrc/lib/observability/types/spanTypes.tssrc/lib/observability/utils/index.tssrc/lib/observability/utils/spanSerializer.tssrc/lib/providers/amazonSagemaker.tssrc/lib/providers/googleVertex.tssrc/lib/rag/pipeline/RAGPipeline.tssrc/lib/rag/ragIntegration.tssrc/lib/rag/retrieval/hybridSearch.tssrc/lib/server/abstract/baseServerAdapter.tssrc/lib/server/middleware/common.tssrc/lib/services/server/ai/observability/instrumentation.tssrc/lib/types/conversationMemoryInterface.tssrc/lib/types/providers.tssrc/lib/utils/pricing.tssrc/lib/utils/ttsProcessor.tssrc/lib/workflow/core/ensembleExecutor.tssrc/lib/workflow/core/judgeScorer.tssrc/lib/workflow/core/workflowRunner.tstest/audit/agent-test-tracing-journey.tstest/continuous-test-suite-context.tstest/continuous-test-suite-evaluation.tstest/continuous-test-suite-mcp-http.tstest/continuous-test-suite-media-gen.tstest/continuous-test-suite-memory.tstest/continuous-test-suite-observability.tstest/continuous-test-suite-ppt.tstest/continuous-test-suite-providers.tstest/continuous-test-suite-rag.tstest/continuous-test-suite-servers.tstest/continuous-test-suite-tracing.tstest/continuous-test-suite-tts.tstest/continuous-test-suite-workflow.tstest/continuous-test-suite.tstest/debug-redis-write.mtstest/unit/evaluation/evaluation.test.tstest/utils/continuousTestHelpers.tstest/zod-schema-test-function.ts
e2d2bd9 to
ef31b7b
Compare
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
murdore
changed the title
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
ef31b7b to
f65acfa
Compare
CI Fix AppliedRoot causeAll 3 CI jobs ( During the rebase onto FixRan
Validation
Note on review commentsThe Copilot and CodeRabbit review comments reference files from the pre-rebase version of this branch (79 of 98 files). The current branch contains 9 files changed from
Most prior inline comments no longer apply to the current diff. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
f65acfa to
2220aea
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
…s, and comprehensive test suite fixes Observability System: - Add 9 span exporters (Langfuse, Datadog, Sentry, PostHog, Arize, Braintrust, Laminar, Langsmith, OTEL) - Add 9 sampling strategies with configurable sampling - Add MetricsAggregator, TokenTracker, SpanProcessor, OTELBridge - Add ExporterRegistry with factory pattern and Promise.allSettled isolation - Add CLI commands: neurolink observability, neurolink telemetry - Add end-to-end span instrumentation across generate/stream/memory/RAG/workflow/PPT/TTS/MCP/context - Add external TracerProvider support with setLangfuseContext, getSpanProcessors, getTracer exports - Add operation name auto-detection and traceNameFormat customization - Add workflow streaming span instrumentation (runWorkflowWithStreaming) SDK Bug Fixes: - Fix structured output guard in GenerationHandler (schema alone now activates structured output) - Fix CLI TTS not wired in generate path - Fix PPT slide type inference regex (word boundaries instead of anchors) - Fix PPT slideRenderers social.map crash (Array.isArray guard) - Fix video polling 404 for global location endpoint - Fix Gemini 3.1 system prompt on global endpoint (prepend as user message) - Fix Gemini 3.1 native path missing conversation history injection - Fix Gemini 2.5 pricing missing + vertex-to-google fallback - Fix span parent-child context propagation via context.with() - Fix traceNameFormat singleton blocking config updates - Fix Redis memory not initializing from constructor config - Fix ConversationMemoryManager missing close() for clean shutdown - Fix enrichWithCost publishing misleading zero input/output costs - Fix evaluation dimension attribute mismatch (relevance|accuracy|completeness) - Fix Arize health check double /v1 URL path - Fix Braintrust metadata leaking sensitive span attributes (PII) - Fix spanSerializer nanoid replaced with crypto.randomBytes for W3C compliance - Fix compactor stage3_failed attribute not recorded on partial failures - Add evaluation exports (RAGASEvaluator, ContextBuilder, RetryManager) Gemini 3.1 Model Support: - Add gemini-3.1-pro, gemini-3.1-flash, gemini-3.1-flash-lite to all enums - Add 1M context windows for all Gemini 3.1 models Test Suite Improvements: - Fix Image Gen Unsupported Provider test (use invalid provider) - Fix Google AI Studio Image Gen return value - Fix CLI Video Generate assertion (exit code, not stdout) - Fix Video Validation test (remove hanging 4K API call) - Add Gemini guard to Zod schema test - Add input/output/traceId assertions to provider observability span test - Add trace hierarchy assertions to memory span test Review Feedback (Cycle 4 - 12 previously unaddressed items): - ragasEvaluator: fix evaluation.dimension attribute to match emitted scores - baseProvider/neurolink: stop publishing zero inputCost/outputCost when only totalCost known - contextCompactor: record compaction.stage3_failed attribute on partial failures - exporterRegistry: use Promise.allSettled for per-exporter isolation in lifecycle methods - workflowRunner: add span instrumentation to runWorkflowWithStreaming - braintrustExporter: remove PII-leaking span.attributes spread from metadata - arizeExporter: fix double /v1/v1/health URL, add batch comment - spanSerializer: replace nanoid with crypto.randomBytes for OTLP-compliant hex IDs - providers test: assert input/output capture and traceId on spans - memory test: assert trace hierarchy (shared traceId/parentSpanId)
2220aea to
d02130c
Compare
Review Feedback AddressedAll 8 review comments have been resolved in the latest push ( Changes Made1. 2. 3. 4. 5. 6. 7. 8. Validation
|
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
🎉 This PR is included in version 9.29.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Test plan