feat(llm-observability): metric and feedback methods #1709
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
skoob13
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds two new methods to the PostHog JavaScript library for capturing AI-related metrics and feedback, enabling straightforward tracking of LLM interactions.
- Added
captureTraceMetric()insrc/posthog-core.tsto track AI metrics with trace ID, name, and value - Added
captureTraceFeedback()insrc/posthog-core.tsto capture user feedback with trace ID - Both methods automatically convert numeric/boolean values to strings for consistent property types
- Added comprehensive test coverage in
src/__tests__/ai.test.tsverifying string conversion and event capture - Methods use existing
$ai_metricevent type with different property structures for metrics vs feedback
2 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
|
|
||
| const setup = (config: Partial<PostHogConfig> = {}, token: string = uuidv7()) => { | ||
| const beforeSendMock = jest.fn().mockImplementation((e) => e) | ||
| const posthog = defaultPostHog().init(token, { ...config, before_send: beforeSendMock }, token)! |
There was a problem hiding this comment.
style: Using the token as both the token and name parameter could cause confusion in tests. Consider using distinct values.
src/__tests__/ai.test.ts
Outdated
| }) | ||
|
|
||
| describe('captureTraceFeedback()', () => { | ||
| it('should capture metric', () => { |
There was a problem hiding this comment.
syntax: Test description says 'should capture metric' but this is testing feedback capture. Should be 'should capture feedback'.
| it('should capture metric', () => { | |
| it('should capture feedback', () => { |
src/posthog-core.ts
Outdated
| captureTraceFeedback(traceId: string | number, userFeedback: string) { | ||
| this.capture('$ai_metric', { | ||
| $ai_trace_id: String(traceId), | ||
| $ai_feedback_text: userFeedback, | ||
| }) |
There was a problem hiding this comment.
style: Consider adding validation for empty/null userFeedback parameter
| captureTraceMetric(traceId: string | number, metricName: string, metricValue: string | number | boolean) { | ||
| this.capture('$ai_metric', { | ||
| $ai_trace_id: String(traceId), | ||
| $ai_metric_name: metricName, | ||
| $ai_metric_value: String(metricValue), | ||
| }) | ||
| } |
There was a problem hiding this comment.
style: Consider adding validation for empty/null metricName parameter
|
Size Change: +2.28 kB (+0.07%) Total Size: 3.3 MB
ℹ️ View Unchanged
|
| captureTraceFeedback(traceId: string | number, userFeedback: string) { | ||
| this.capture('$ai_feedback', { | ||
| $ai_trace_id: String(traceId), | ||
| $ai_feedback_text: userFeedback, |
There was a problem hiding this comment.
just thinking here do we want to allow the user to also provide a label?
For example, a common pattern is to get a thumbs up or thumbs down and then ask why. it would be great to capture the text with the action
There was a problem hiding this comment.
My gut feeling is that you typically want to capture a single piece of feedback, but I can imagine cases when this is useful, such as multi-step agents. Should we do that in this PR?
Problem
It should be straightforward to capture a LLM metric or feedback. Mirrors PostHog/posthog-js-lite#365.
Changes
Checklist