Fix openAI telemetry response attributes use the request namespace type - #7126
Conversation
fubhy
added
the
audit
🦋 Changeset detectedLatest commit: 876e03d The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This PR currently contains only the reproduction test; the implementation fix it describes is missing. The added test is correct and fails as expected, but the branch needs the source change before it can merge.
Reviewed changes
This PR adds a type-level regression test in packages/ai/openai/typetest/OpenAiTelemetry.tst.ts asserting that OpenAiTelemetryAttributes accepts gen_ai.openai.response.system_fingerprint and rejects the gen_ai.openai.request.* variant. I verified the test fails against the current implementation.
⚠️ Implementation fix is missing
packages/ai/openai/src/OpenAiTelemetry.ts:32 still prefixes ResponseAttributes with "gen_ai.openai.request" instead of "gen_ai.openai.response". The runtime helpers on lines 126-127 already emit the correct response namespace, so the type-level alias is inconsistent with runtime behavior. The fix is the single-line change noted in the PR body.
⚠️ Same bug in @effect/ai-openai-compat
packages/ai/openai-compat/src/OpenAiTelemetry.ts:31 has the identical copy-paste error. Since the two modules mirror each other and both runtime paths already use "gen_ai.openai.response", the compat package should be fixed and covered by an analogous type test.
ℹ️ Changeset
Once the source changes land, add a changeset for the affected packages (they are in the same fixed release group).
@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The latest commit completes the fix started in the first commit and resolves all feedback from the previous review.
- Applied the source fix to both packages —
OpenAiTelemetryAttributesin@effect/ai-openaiand@effect/ai-openai-compatnow prefixesResponseAttributeswith"gen_ai.openai.response"instead of"gen_ai.openai.request", aligning the type with the runtime helpers. - Added matching type-level regression tests in
packages/ai/openai/typetest/OpenAiTelemetry.tst.tsandpackages/ai/openai-compat/typetest/OpenAiTelemetry.tst.ts, asserting thatgen_ai.openai.response.system_fingerprintis assignable andgen_ai.openai.request.system_fingerprintis not. - Added a patch changeset covering both packages in the same release group.
Validation run: type tests, package runtime tests, package check, and repo lint all pass.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
There was a problem hiding this comment.
Important
The new commit removes the type-level regression tests that verified exactly the bug this PR fixes, leaving the fix without automated coverage.
Reviewed changes
Since the last Pullfrog review (04e63764), the only change is commit 876e03d:
- Removed both type-level regression tests — deleted
packages/ai/openai/typetest/OpenAiTelemetry.tst.tsandpackages/ai/openai-compat/typetest/OpenAiTelemetry.tst.ts, which assertedgen_ai.openai.response.*is assignable toOpenAiTelemetryAttributesand thegen_ai.openai.request.*variant is not.
⚠️ Removing the regression tests drops the only coverage for this fix
This PR is a pure type-level correction: the source change swaps the ResponseAttributes prefix from gen_ai.openai.request to gen_ai.openai.response in OpenAiTelemetry.ts (both packages). The runtime package tests exercise OpenAI API behavior and cannot detect a mistake in the telemetry attribute type namespace, so the deleted typetests were the sole automated guard for the exact bug being fixed.
The repo's tstyche pipeline would run these tests — tstyche.json matches packages/*/*/typetest/**/*.tst.*, and openai-compat retains a sibling OpenAiConfig.tst.ts in the same directory. Without a regression test, a future change re-introducing gen_ai.openai.request for response attributes would ship silently.
Technical details
# Re-add type-level regression coverage
## Affected sites
- packages/ai/openai/typetest/OpenAiTelemetry.tst.ts — deleted in 876e03d
- packages/ai/openai-compat/typetest/OpenAiTelemetry.tst.ts — deleted in 876e03d
## Required outcome
- Restore at least one type-level test asserting `{ "gen_ai.openai.response.system_fingerprint" }` is assignable to `OpenAiTelemetryAttributes` and `{ "gen_ai.openai.request.system_fingerprint" }` is not (or the equivalent for `service_tier`). It must fail against the buggy `.request` prefix and pass with the `.response` prefix.
- Run `pnpm test-types packages/ai/openai/typetest/OpenAiTelemetry.tst.ts packages/ai/openai-compat/typetest/OpenAiTelemetry.tst.ts` to confirm.
## Open questions for the human
- If the tests were removed because they did not actually run or were invalid, please note why — the sibling `OpenAiConfig.tst.ts` in `openai-compat` indicates typetests do run for these packages.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
2 participants
Summary
gen_ai.openai.responsenamespace emitted by the runtime helpers.@effect/ai-openaiand@effect/ai-openai-compat.Validation
pnpm test --run packages/ai/openai/test packages/ai/openai-compat/testpnpm --filter @effect/ai-openai --filter @effect/ai-openai-compat checkpnpm checkpnpm lintAudit provenance
b206fa5d7655c1634c9993410a9203f6616a5ca2relsem-openai-telemetry-response-namespaceCloses EFF-560