feat(sdk): add PostHog usage tracking#1999
Merged
cherkanovart merged 3 commits intomainfrom Feb 23, 2026
Merged
Conversation
Track SDK method calls (localize*, recognizeLocale) with start/success/error events via PostHog. Identity is resolved through /whoami endpoint with fallback to hashed API key. Respects DO_NOT_TRACK=1 for opt-out. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds a PostHog-based observability module and instruments SDK localization and recognition methods to emit start/success/error tracking events; it also adds tests, a changelog entry, and dependency/config updates. Public method signatures remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK as SDK Method
participant Tracking as Observability.trackEvent
participant Identity as getDistinctId
participant Whoami as Whoami API
participant PostHog
Client->>SDK: call localize*/recognize*
SDK->>Tracking: trackEvent(LOCALIZE/RECOGNIZE_START, props)
Tracking->>Identity: resolveIdentity(apiKey, apiUrl)
Identity->>Whoami: GET /whoami (Bearer)
Whoami-->>Identity: email or error
Identity-->>Tracking: distinct_id + distinct_id_source
Tracking->>PostHog: dynamic import + capture(event, properties...)
PostHog-->>Tracking: acknowledged
SDK->>SDK: perform localization/recognition work
alt success
SDK->>Tracking: trackEvent(..._SUCCESS, props)
else error
SDK->>Tracking: trackEvent(..._ERROR, { errorMessage })
SDK-->>Client: rethrow/error
end
Tracking->>PostHog: capture(event, properties...)
PostHog-->>Tracking: acknowledged
SDK-->>Client: return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/sdk/src/index.ts (1)
525-531: Consider expanding localizable attributes for additional elements.The current
LOCALIZABLE_ATTRIBUTEScoversmeta,img,input, andatags. Consider whetherbutton(title),textarea(placeholder),label, orarea(alt) should also be included for comprehensive HTML localization.💡 Potential additions
const LOCALIZABLE_ATTRIBUTES: Record<string, string[]> = { meta: ["content"], img: ["alt"], input: ["placeholder"], a: ["title"], + button: ["title"], + textarea: ["placeholder"], + area: ["alt"], };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/index.ts` around lines 525 - 531, LOCALIZABLE_ATTRIBUTES currently lists only meta/img/input/a; expand this mapping to include other commonly localized elements such as button (title and innerText if applicable), textarea (placeholder and value/innerText), label (innerText), and area (alt) so they get picked up by localization logic; update the LOCALIZABLE_ATTRIBUTES constant to add keys "button", "textarea", "label", and "area" with the appropriate attribute arrays, ensure any logic that reads LOCALIZABLE_ATTRIBUTES (the code that iterates this constant) also handles innerText/localizable node content if needed, and add or update unit tests to cover these new tag mappings and verify UNLOCALIZABLE_TAGS behavior remains unchanged.packages/sdk/src/utils/observability.spec.ts (1)
42-42: Consider usingvi.waitForor flushing promises instead of arbitrary timeouts.The 200ms delays throughout the tests (lines 42, 66, 86, 106, 109, 127, 130) could cause flaky behavior in slow CI environments or race conditions. Vitest provides
vi.waitForor you can flush promises more reliably.♻️ Example using vi.waitFor
- await new Promise((r) => setTimeout(r, 200)); - - expect(capture).toHaveBeenCalledWith( + await vi.waitFor(() => { + expect(capture).toHaveBeenCalledWith( expect.objectContaining({ distinctId: "user@test.com", ... }), - ); + ); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/utils/observability.spec.ts` at line 42, Replace each literal delay "await new Promise((r) => setTimeout(r, 200));" in the observability tests with deterministic waiting: use vi.waitFor(() => /* condition that signals the async work completed, e.g. mock called or state updated */) or call a flushPromises helper (e.g. await flushPromises() implemented via Promise.resolve/queueMicrotask) and import vi if needed; update all occurrences matching that exact expression so tests wait on observable conditions instead of a fixed timeout.packages/sdk/src/utils/observability.ts (2)
47-65: Consider reusing the PostHog client instance.Creating a new
PostHogclient for every event and immediately shutting it down adds overhead. For improved efficiency, consider using a singleton pattern with a module-level client that flushes on process exit or after a batch of events.♻️ Suggested singleton pattern
+let posthogClient: PostHog | null = null; + +async function getPostHogClient(): Promise<PostHog> { + if (!posthogClient) { + const { PostHog } = await import("posthog-node"); + posthogClient = new PostHog(POSTHOG_API_KEY, { + host: POSTHOG_HOST, + flushAt: 10, + flushInterval: 10000, + }); + } + return posthogClient; +} + async function resolveIdentityAndCapture( apiKey: string, apiUrl: string, event: string, properties?: Record<string, any>, ) { const identityInfo = await getDistinctId(apiKey, apiUrl); // ... debug logging ... - const { PostHog } = await import("posthog-node"); - const posthog = new PostHog(POSTHOG_API_KEY, { - host: POSTHOG_HOST, - flushAt: 1, - flushInterval: 0, - }); + const posthog = await getPostHogClient(); await posthog.capture({ distinctId: identityInfo.distinct_id, event, properties: { ...properties, tracking_version: TRACKING_VERSION, sdk_package: SDK_PACKAGE, distinct_id_source: identityInfo.distinct_id_source, }, }); - - await posthog.shutdown(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/utils/observability.ts` around lines 47 - 65, The code currently instantiates a new PostHog client for every tracked event (new PostHog(...), posthog.capture(...), posthog.shutdown()), which is wasteful; refactor to create a module-level singleton PostHog instance (e.g., export or keep a private cached PostHog client) and reuse it across calls to your tracking function in observability.ts, call posthog.capture(...) on the singleton, and only call shutdown() once on process exit (or after batching), ensuring initialization is lazy and guarded so the same PostHog object is reused instead of recreating and shutting it down per event.
75-96: Add timeout to prevent indefinite hangs.The fetch call to
/whoamihas no timeout. If the server is unresponsive, this could delay event tracking indefinitely. Consider adding anAbortControllerwith a reasonable timeout (e.g., 5 seconds).⏱️ Proposed fix with timeout
try { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); const res = await fetch(`${apiUrl}/whoami`, { method: "POST", headers: { Authorization: `Bearer ${apiKey}`, - ContentType: "application/json", + "Content-Type": "application/json", }, + signal: controller.signal, }); + clearTimeout(timeoutId); if (res.ok) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/utils/observability.ts` around lines 75 - 96, The fetch to `${apiUrl}/whoami` in observability.ts can hang indefinitely; wrap the request in an AbortController with a short timeout (e.g., 5s) and pass controller.signal into fetch, clearing the timer on success or error; ensure you abort the controller when the timeout elapses so the catch branch handles the failure. Apply this change around the existing try block where the fetch is performed (the POST to `/whoami`) and keep the existing response handling and identityCache.set logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/src/index.ts`:
- Around line 711-718: The code returns jsonResponse.locale without validation;
update the method in packages/sdk/src/index.ts to check that jsonResponse &&
typeof jsonResponse.locale === "string" and that the value is a valid LocaleCode
(e.g., compare against your LocaleCode enum or a isValidLocale helper) before
returning; if invalid or missing, call trackEvent with
TRACKING_EVENTS.RECOGNIZE_FAILURE (or log/throw a clear error) and either throw
an Error or return a safe default locale, ensuring callers never receive
undefined from jsonResponse.locale.
- Around line 465-468: The mapping that builds result is fragile because it
assumes localized keys are in "chat_N" form and uses parseInt(key.split("_")[1])
which can yield NaN and index into chat incorrectly; update the mapping in
packages/sdk/src/index.ts (the block that produces result from localized) to
defensively parse and validate the index: extract the suffix safely (e.g., split
and check length), use Number(...) and isFinite/isNaN to verify a valid integer,
fall back to a safe default or skip entries when the index is invalid, and
ensure you reference chat[index].name only after confirming index is a valid
array index to avoid undefined lookups and runtime errors.
- Around line 407-414: The code returns Object.values(result) which can misorder
items for 10+ keys; change the return to preserve input order by mapping the
original mapped keys to the result (e.g., use the same mapped array/keys used to
call _localizeRaw and return mapped.map(k => result[k]) or adjust _localizeRaw
to return an array in the same order); update the return in the function that
calls _localizeRaw (where result, mapped, params, trackEvent, and
TRACKING_EVENTS.LOCALIZE_SUCCESS are used) so the output order matches the input
order.
In `@packages/sdk/src/utils/observability.ts`:
- Around line 78-82: The headers object in
packages/sdk/src/utils/observability.ts uses the wrong header key `ContentType`;
change it to the correct HTTP header name `Content-Type` (including the hyphen
and proper casing) in the headers literal where Authorization is set so the
server recognizes the content type; ensure any other places building the same
headers object (e.g., the request/telemetry sending function) are updated
similarly.
In `@packages/sdk/src/utils/tracking-events.ts`:
- Line 12: The SDK_PACKAGE constant is set to the wrong package name; update the
SDK_PACKAGE symbol in tracking-events.ts to match the package.json package name
(include the underscore) so it reads the actual package name `@lingo.dev/_sdk`,
ensuring any telemetry uses the correct identifier.
---
Nitpick comments:
In `@packages/sdk/src/index.ts`:
- Around line 525-531: LOCALIZABLE_ATTRIBUTES currently lists only
meta/img/input/a; expand this mapping to include other commonly localized
elements such as button (title and innerText if applicable), textarea
(placeholder and value/innerText), label (innerText), and area (alt) so they get
picked up by localization logic; update the LOCALIZABLE_ATTRIBUTES constant to
add keys "button", "textarea", "label", and "area" with the appropriate
attribute arrays, ensure any logic that reads LOCALIZABLE_ATTRIBUTES (the code
that iterates this constant) also handles innerText/localizable node content if
needed, and add or update unit tests to cover these new tag mappings and verify
UNLOCALIZABLE_TAGS behavior remains unchanged.
In `@packages/sdk/src/utils/observability.spec.ts`:
- Line 42: Replace each literal delay "await new Promise((r) => setTimeout(r,
200));" in the observability tests with deterministic waiting: use vi.waitFor(()
=> /* condition that signals the async work completed, e.g. mock called or state
updated */) or call a flushPromises helper (e.g. await flushPromises()
implemented via Promise.resolve/queueMicrotask) and import vi if needed; update
all occurrences matching that exact expression so tests wait on observable
conditions instead of a fixed timeout.
In `@packages/sdk/src/utils/observability.ts`:
- Around line 47-65: The code currently instantiates a new PostHog client for
every tracked event (new PostHog(...), posthog.capture(...),
posthog.shutdown()), which is wasteful; refactor to create a module-level
singleton PostHog instance (e.g., export or keep a private cached PostHog
client) and reuse it across calls to your tracking function in observability.ts,
call posthog.capture(...) on the singleton, and only call shutdown() once on
process exit (or after batching), ensuring initialization is lazy and guarded so
the same PostHog object is reused instead of recreating and shutting it down per
event.
- Around line 75-96: The fetch to `${apiUrl}/whoami` in observability.ts can
hang indefinitely; wrap the request in an AbortController with a short timeout
(e.g., 5s) and pass controller.signal into fetch, clearing the timer on success
or error; ensure you abort the controller when the timeout elapses so the catch
branch handles the failure. Apply this change around the existing try block
where the fetch is performed (the POST to `/whoami`) and keep the existing
response handling and identityCache.set logic unchanged.
Comment on lines
+407
to
+414
| const result = await this._localizeRaw(mapped, params); | ||
| trackEvent( | ||
| this.config.apiKey, | ||
| this.config.apiUrl, | ||
| TRACKING_EVENTS.LOCALIZE_SUCCESS, | ||
| trackProps, | ||
| ); | ||
| return Object.values(result); |
Contributor
There was a problem hiding this comment.
Potential ordering issue with arrays of 10+ elements.
Object.values(result) relies on object key insertion order. If the API returns keys in a different order, or if there are 10+ items (where item_10 lexicographically sorts before item_2), the result order may not match the input.
🛡️ Proposed fix to ensure order preservation
const result = await this._localizeRaw(mapped, params);
trackEvent(
this.config.apiKey,
this.config.apiUrl,
TRACKING_EVENTS.LOCALIZE_SUCCESS,
trackProps,
);
- return Object.values(result);
+ return strings.map((_, i) => result[`item_${i}`]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = await this._localizeRaw(mapped, params); | |
| trackEvent( | |
| this.config.apiKey, | |
| this.config.apiUrl, | |
| TRACKING_EVENTS.LOCALIZE_SUCCESS, | |
| trackProps, | |
| ); | |
| return Object.values(result); | |
| const result = await this._localizeRaw(mapped, params); | |
| trackEvent( | |
| this.config.apiKey, | |
| this.config.apiUrl, | |
| TRACKING_EVENTS.LOCALIZE_SUCCESS, | |
| trackProps, | |
| ); | |
| return strings.map((_, i) => result[`item_${i}`]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/index.ts` around lines 407 - 414, The code returns
Object.values(result) which can misorder items for 10+ keys; change the return
to preserve input order by mapping the original mapped keys to the result (e.g.,
use the same mapped array/keys used to call _localizeRaw and return mapped.map(k
=> result[k]) or adjust _localizeRaw to return an array in the same order);
update the return in the function that calls _localizeRaw (where result, mapped,
params, trackEvent, and TRACKING_EVENTS.LOCALIZE_SUCCESS are used) so the output
order matches the input order.
Comment on lines
+465
to
+468
| const result = Object.entries(localized).map(([key, value]) => ({ | ||
| name: chat[parseInt(key.split("_")[1])].name, | ||
| text: value, | ||
| })); |
Contributor
There was a problem hiding this comment.
Fragile key parsing assumes specific response format.
The code assumes API response keys follow the chat_N pattern. If the API returns keys in a different format, parseInt(key.split("_")[1]) could produce NaN, causing chat[NaN] to be undefined.
🛡️ Proposed defensive fix
- const result = Object.entries(localized).map(([key, value]) => ({
- name: chat[parseInt(key.split("_")[1])].name,
- text: value,
- }));
+ const result = chat.map((msg, i) => ({
+ name: msg.name,
+ text: localized[`chat_${i}`] ?? msg.text,
+ }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const result = Object.entries(localized).map(([key, value]) => ({ | |
| name: chat[parseInt(key.split("_")[1])].name, | |
| text: value, | |
| })); | |
| const result = chat.map((msg, i) => ({ | |
| name: msg.name, | |
| text: localized[`chat_${i}`] ?? msg.text, | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/index.ts` around lines 465 - 468, The mapping that builds
result is fragile because it assumes localized keys are in "chat_N" form and
uses parseInt(key.split("_")[1]) which can yield NaN and index into chat
incorrectly; update the mapping in packages/sdk/src/index.ts (the block that
produces result from localized) to defensively parse and validate the index:
extract the suffix safely (e.g., split and check length), use Number(...) and
isFinite/isNaN to verify a valid integer, fall back to a safe default or skip
entries when the index is invalid, and ensure you reference chat[index].name
only after confirming index is a valid array index to avoid undefined lookups
and runtime errors.
Comment on lines
+711
to
+718
| const jsonResponse = await response.json(); | ||
| trackEvent( | ||
| this.config.apiKey, | ||
| this.config.apiUrl, | ||
| TRACKING_EVENTS.RECOGNIZE_SUCCESS, | ||
| trackProps, | ||
| ); | ||
| return jsonResponse.locale; |
Contributor
There was a problem hiding this comment.
Missing validation of API response.
jsonResponse.locale is returned directly without validating that it exists or is a valid LocaleCode. If the API returns an unexpected response, this could propagate undefined to callers.
🛡️ Proposed fix with validation
const jsonResponse = await response.json();
+ const locale = localeCodeSchema.safeParse(jsonResponse.locale);
+ if (!locale.success) {
+ throw new Error(`Invalid locale response: ${JSON.stringify(jsonResponse)}`);
+ }
trackEvent(
this.config.apiKey,
this.config.apiUrl,
TRACKING_EVENTS.RECOGNIZE_SUCCESS,
trackProps,
);
- return jsonResponse.locale;
+ return locale.data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/index.ts` around lines 711 - 718, The code returns
jsonResponse.locale without validation; update the method in
packages/sdk/src/index.ts to check that jsonResponse && typeof
jsonResponse.locale === "string" and that the value is a valid LocaleCode (e.g.,
compare against your LocaleCode enum or a isValidLocale helper) before
returning; if invalid or missing, call trackEvent with
TRACKING_EVENTS.RECOGNIZE_FAILURE (or log/throw a clear error) and either throw
an Error or return a safe default locale, ensuring callers never receive
undefined from jsonResponse.locale.
AndreyHirsa
approved these changes
Feb 20, 2026
Fix SDK_PACKAGE constant to match actual package name "@lingo.dev/_sdk" and fix malformed "ContentType" header to proper "Content-Type" in whoami request headers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/sdk/src/index.ts (3)
465-468: Fragile key parsing assumes specific response format.The code assumes API response keys follow the
chat_Npattern. If the API returns keys differently,parseInt(key.split("_")[1])could produceNaN.🛡️ Proposed defensive fix
- const result = Object.entries(localized).map(([key, value]) => ({ - name: chat[parseInt(key.split("_")[1])].name, - text: value, - })); + const result = chat.map((msg, i) => ({ + name: msg.name, + text: localized[`chat_${i}`] ?? msg.text, + }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/index.ts` around lines 465 - 468, The mapping that builds result from localized is fragile because it assumes keys follow "chat_N" and uses parseInt(key.split("_")[1]) directly; update the logic in the result creation to extract the index with a defensive check (e.g., use a regex to capture the numeric suffix or explicitly check split length), validate Number.isInteger(index) and that index is within bounds of the chat array before accessing chat[index].name, and if the key doesn't match or index is invalid either skip that entry or use a safe default (e.g., use chat[0].name or a placeholder) so you never pass NaN into chat[...] access.
711-718: Missing validation of API response.
jsonResponse.localeis returned directly without validating that it exists or is a validLocaleCode. If the API returns an unexpected response, this could propagateundefinedto callers.🛡️ Proposed fix with validation
const jsonResponse = await response.json(); + const locale = localeCodeSchema.safeParse(jsonResponse.locale); + if (!locale.success) { + throw new Error(`Invalid locale response: ${JSON.stringify(jsonResponse)}`); + } trackEvent( this.config.apiKey, this.config.apiUrl, TRACKING_EVENTS.RECOGNIZE_SUCCESS, trackProps, ); - return jsonResponse.locale; + return locale.data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/index.ts` around lines 711 - 718, The code returns jsonResponse.locale without validation which can propagate undefined; update the handler that calls response.json() (the block using jsonResponse, trackEvent and TRACKING_EVENTS.RECOGNIZE_SUCCESS) to validate that jsonResponse.locale exists and is a valid LocaleCode (or one of the expected values) before returning it, and if it is missing/invalid either throw a descriptive error or return a safe default; use the LocaleCode type/enum or a small validation helper to perform the check and include the invalid value in the error/log for easier debugging.
407-414: Potential ordering issue with arrays of 10+ elements.
Object.values(result)relies on object key insertion order. For keys likeitem_0throughitem_10+, lexicographic ordering (item_10beforeitem_2) may not match input order.🛡️ Proposed fix to ensure order preservation
const result = await this._localizeRaw(mapped, params); trackEvent( this.config.apiKey, this.config.apiUrl, TRACKING_EVENTS.LOCALIZE_SUCCESS, trackProps, ); - return Object.values(result); + return strings.map((_, i) => result[`item_${i}`]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/index.ts` around lines 407 - 414, The returned ordering can break for keys like "item_0".."item_10" because Object.values(result) uses object key order; update the code in the function that calls this._localizeRaw (the block that calls trackEvent and returns Object.values(result)) to preserve the original input order by mapping the original input identifiers to the result instead of using Object.values: either change _localizeRaw to return an array in the correct order or, if _localizeRaw must return a keyed object, compute and return mapped.map(key => result[key]) (using the same "mapped" input variable) so the final returned array matches the input order exactly. Ensure this change is applied where Object.values(result) is currently used (near the call to this._localizeRaw and TRACKING_EVENTS.LOCALIZE_SUCCESS).
🧹 Nitpick comments (2)
packages/sdk/src/utils/observability.ts (2)
83-92: Add defensive handling for JSON parsing failure.If the server returns
200 OKbut with invalid JSON,res.json()will throw. Consider wrapping in try-catch to fall through to the hash fallback gracefully.🛡️ Proposed defensive fix
if (res.ok) { - const payload = await res.json(); - if (payload?.email) { - const identity: IdentityInfo = { - distinct_id: payload.email, - distinct_id_source: "email", - }; - identityCache.set(apiKey, identity); - return identity; + try { + const payload = await res.json(); + if (payload?.email) { + const identity: IdentityInfo = { + distinct_id: payload.email, + distinct_id_source: "email", + }; + identityCache.set(apiKey, identity); + return identity; + } + } catch { + // Fall through to API key hash } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/utils/observability.ts` around lines 83 - 92, When handling a 200 response in the block that checks res.ok, wrap the await res.json() call in a try-catch so JSON parse errors don't throw and instead allow the function to fall back to the existing hash fallback; if parsing succeeds and payload?.email exists, proceed to construct IdentityInfo, set identityCache.set(apiKey, identity) and return identity, but on any JSON parse error quietly ignore the payload and continue to the hash-based identity generation path.
47-65: Consider reusing the PostHog client instead of creating one per event.Creating a new
PostHogclient and callingshutdown()for every tracked event adds latency and overhead. For high-frequency SDK usage, this could degrade performance.Consider initializing a singleton client lazily and reusing it, or batching events before shutdown.
♻️ Suggested approach
let posthogClient: PostHog | null = null; async function getPostHogClient(): Promise<PostHog> { if (!posthogClient) { const { PostHog } = await import("posthog-node"); posthogClient = new PostHog(POSTHOG_API_KEY, { host: POSTHOG_HOST, }); } return posthogClient; } // Then use getPostHogClient() in resolveIdentityAndCapture // and handle graceful shutdown on process exit if needed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/utils/observability.ts` around lines 47 - 65, The current code creates a new PostHog client for every event and calls shutdown() each time; change this to a lazily-initialized singleton PostHog client (e.g., add a module-level posthogClient and a getPostHogClient() that imports PostHog-node and constructs the client once) and update the code paths that call posthog.capture (such as resolveIdentityAndCapture / whichever function contains the capture call) to use getPostHogClient() instead of creating a new PostHog instance; remove per-event posthog.shutdown() and instead perform a single graceful flush/shutdown on process exit (or expose a shutdown helper) so events are batched and latency is reduced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/sdk/src/index.ts`:
- Around line 465-468: The mapping that builds result from localized is fragile
because it assumes keys follow "chat_N" and uses parseInt(key.split("_")[1])
directly; update the logic in the result creation to extract the index with a
defensive check (e.g., use a regex to capture the numeric suffix or explicitly
check split length), validate Number.isInteger(index) and that index is within
bounds of the chat array before accessing chat[index].name, and if the key
doesn't match or index is invalid either skip that entry or use a safe default
(e.g., use chat[0].name or a placeholder) so you never pass NaN into chat[...]
access.
- Around line 711-718: The code returns jsonResponse.locale without validation
which can propagate undefined; update the handler that calls response.json()
(the block using jsonResponse, trackEvent and TRACKING_EVENTS.RECOGNIZE_SUCCESS)
to validate that jsonResponse.locale exists and is a valid LocaleCode (or one of
the expected values) before returning it, and if it is missing/invalid either
throw a descriptive error or return a safe default; use the LocaleCode type/enum
or a small validation helper to perform the check and include the invalid value
in the error/log for easier debugging.
- Around line 407-414: The returned ordering can break for keys like
"item_0".."item_10" because Object.values(result) uses object key order; update
the code in the function that calls this._localizeRaw (the block that calls
trackEvent and returns Object.values(result)) to preserve the original input
order by mapping the original input identifiers to the result instead of using
Object.values: either change _localizeRaw to return an array in the correct
order or, if _localizeRaw must return a keyed object, compute and return
mapped.map(key => result[key]) (using the same "mapped" input variable) so the
final returned array matches the input order exactly. Ensure this change is
applied where Object.values(result) is currently used (near the call to
this._localizeRaw and TRACKING_EVENTS.LOCALIZE_SUCCESS).
---
Nitpick comments:
In `@packages/sdk/src/utils/observability.ts`:
- Around line 83-92: When handling a 200 response in the block that checks
res.ok, wrap the await res.json() call in a try-catch so JSON parse errors don't
throw and instead allow the function to fall back to the existing hash fallback;
if parsing succeeds and payload?.email exists, proceed to construct
IdentityInfo, set identityCache.set(apiKey, identity) and return identity, but
on any JSON parse error quietly ignore the payload and continue to the
hash-based identity generation path.
- Around line 47-65: The current code creates a new PostHog client for every
event and calls shutdown() each time; change this to a lazily-initialized
singleton PostHog client (e.g., add a module-level posthogClient and a
getPostHogClient() that imports PostHog-node and constructs the client once) and
update the code paths that call posthog.capture (such as
resolveIdentityAndCapture / whichever function contains the capture call) to use
getPostHogClient() instead of creating a new PostHog instance; remove per-event
posthog.shutdown() and instead perform a single graceful flush/shutdown on
process exit (or expose a shutdown helper) so events are batched and latency is
reduced.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/sdk/src/index.tspackages/sdk/src/utils/observability.spec.tspackages/sdk/src/utils/observability.tspackages/sdk/src/utils/tracking-events.spec.tspackages/sdk/src/utils/tracking-events.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/sdk/src/utils/observability.spec.ts
- packages/sdk/src/utils/tracking-events.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
localizeObject,localizeText,localizeChat,localizeHtml,localizeStringArray,recognizeLocale) with start/success/error events via PostHog/whoamiendpoint (email) with fallback to hashed API keyDO_NOT_TRACK=1env var for opt-outTest plan
pnpm --filter @lingo.dev/_sdk testpassesDEBUG=true/whoami(email: max@replexica.com)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores