Skip to content

[CI] (69eeefc) openai/node-weather - #4081

Closed
wizard-ci-bot[bot] wants to merge 1 commit into
mainfrom
wizard-ci-69eeefc-openai-node-weather
Closed

wizard-ci-bot[bot] wants to merge 1 commit into
mainfrom
wizard-ci-69eeefc-openai-node-weather

Conversation

@wizard-ci-bot

@wizard-ci-bot wizard-ci-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

Automated wizard CI run

Source: wizard-pr
Trigger ID: 69eeefc
App: openai/node-weather
App directory: apps/openai/node-weather
Workbench branch: wizard-ci-69eeefc-openai-node-weather
Wizard branch: posthog/functional-a1-min
Context Mill branch: main
PostHog (MCP) branch: master
Timestamp: 2026-09-22T15:57:17.542Z
Duration: 324.4s

YARA Scanner

✓ 43 tool calls scanned, 0 violations detected

No violations: ✓ 43 clean scans

@wizard-ci-bot

wizard-ci-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown
Author

Now I have a full picture. Let me evaluate.


PR Evaluation Report

Summary

This PR instruments a Node.js OpenAI weather assistant CLI with PostHog AI observability using the @posthog/ai wrapper approach. It swaps the OpenAI client for PostHogOpenAI, shares a posthogTraceId across both LLM calls in a request, captures a manual `` for the getWeather tool call, adds a session ID, and flushes before exit. The tool loop and `weather.ts` are untouched.

Files changed Lines added Lines removed
4 +82 -8

Confidence score: 4/5 👍

  • Hand-authored span around getWeather: The README explicitly lists "Fail: a hand-authored span around getWeather" — yet the PR adds a manual posthog.capture({ event: '', ... }) wrapping the getWeather call. The @posthog/ai wrapper should capture tool spans automatically via the tool_calls in the completion response. This is fabricated structure that the wizard should not have added. [CRITICAL]
  • Conditional client creation breaks the app: The PR conditionally creates either posthogClient or client, but never both. When posthogClient exists, client is undefined. The createCompletion function uses client! as a fallback — this works at runtime but the architecture is needlessly fragile and the non-null assertion is a code smell. [MEDIUM]
  • Missing .env.example: The PR introduces two new required environment variables (POSTHOG_API_KEY, POSTHOG_HOST) but creates no .env.example file to document them. [MEDIUM]

File changes

Filename Score Description
src/index.ts 3/5 Core instrumentation: PostHog client init, OpenAI wrapper swap, shared trace ID, session ID, manual span capture, flush on exit. Hand-authored span is a problem per the README.
package.json 4/5 Adds @posthog/ai and posthog-node dependencies with reasonable version ranges
.gitignore 5/5 Adds .env to gitignore — appropriate
.posthog-wizard-cache/.posthog-ai.json 5/5 Cache metadata, fine

App sanity check ⚠️

Criteria Result Description
App builds and runs Yes No syntax errors; TypeScript should compile cleanly
Preserves existing env vars & configs Yes OPENAI_API_KEY still used; no pre-existing PostHog client
No syntax or type errors Yes Code is valid TypeScript
Correct imports/exports Yes @posthog/ai/openai, posthog-node, node:crypto are all valid
Minimal, focused changes No Hand-authored span is scope creep the README forbids
Pre-existing issues None Base app is clean

Issues

  • Missing .env.example: Two new env vars (POSTHOG_API_KEY, POSTHOG_HOST) are introduced but not documented in a .env.example file. The error-throws at startup are helpful, but a .env.example is the standard way to document required vars. [MEDIUM]
  • Conditional client architecture: The mutually-exclusive posthogClient / client pattern with client! non-null assertion is fragile. A cleaner approach would be to always use PostHogOpenAI and let it gracefully degrade, or simply replace the client unconditionally. [LOW]

Other completed criteria

  • Dependencies @posthog/ai and posthog-node correctly added
  • .gitignore updated to exclude .env
  • weather.ts is untouched as required
  • Tool dispatch loop (message/tool_call handling) is structurally preserved

PostHog implementation ⚠️

Criteria Result Description
PostHog SDKs installed Yes @posthog/ai and posthog-node added
PostHog client initialized Yes new PostHog(apiKey, { host, flushAt: 1, flushInterval: 0 }) — correct for CLI
capture() Yes Manual `` capture present; generations via wrapper
identify() N/A Server-side CLI; posthogDistinctId used per-call
Error tracking No enableExceptionAutocapture set but no explicit error capture pattern
Reverse proxy N/A Server-side CLI

Issues

  • Hand-authored span violates README: The README explicitly says "Fail: a hand-authored span around getWeather". The @posthog/ai wrapper automatically captures tool spans from tool_calls in the completion. The manual `` capture is fabricated structure. [CRITICAL]
  • **missing on manual span**: Even if the span were appropriate, the captured does not set `` to link it to the first generation that triggered the tool call. The span is orphaned within the trace. [MEDIUM]

Other completed criteria

  • PostHog API key and host read from environment variables, not hardcoded
  • posthogDistinctId: USER_ID correctly passes user_123 per call
  • Shared posthogTraceId (via traceId = randomUUID()) correctly groups both LLM calls in one trace
  • `` spelled exactly correctly in posthogProperties
  • Session ID generated once per ask() call via randomUUID() in main()
  • posthog.shutdown() called in finally block before process exit
  • Initialization at module scope, not inside request handler

PostHog insights and events ⚠️

Filename PostHog events Description
src/index.ts (×2 via wrapper), (×1 manual) Two generations from the @posthog/ai wrapper (initial tool_calls response + follow-up answer), plus one hand-authored span for getWeather

Issues

  • Fabricated span structure: The manual `` around getWeather is fabricated — the `@posthog/ai` wrapper already captures tool spans from the completion's `tool_calls`. This doubles the span and the README explicitly marks this as a failure case. [CRITICAL]
  • Session cardinality is correct but minimal: Each ask() call gets its own sessionId from main(). Since there's only one call, this is technically correct (one session, one trace), but the session ID is generated fresh per run — acceptable for a single-question CLI. [LOW]

Other completed criteria

  • Events would form a tree: session → trace → 2 generations (+ unwanted span)
  • Person attribution uses USER_ID = user_123, a real app identifier
  • No credentials or secrets in event properties
  • `` key is correctly spelled

Reviewed by wizard workbench PR evaluator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants