Open
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 introduces PostHog-based usage tracking to the SDK by adding a new observability module that captures method execution events (start, success, error) for localization and recognition operations without altering public method signatures or control flow. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SDK as SDK Method
participant Tracking as trackEvent
participant Identity as getDistinctId
participant API as Whoami API
participant PostHog
Client->>SDK: Call localizeObject()
SDK->>Tracking: trackEvent(LOCALIZE_START)
Tracking->>Identity: getDistinctId(apiKey, apiUrl)
Identity->>API: Fetch whoami
API-->>Identity: User email or error
Identity-->>Tracking: Return distinctId & source
Tracking->>PostHog: capture(event, properties)
PostHog-->>Tracking: Acknowledged
SDK->>SDK: Execute localization logic
alt Success
SDK->>Tracking: trackEvent(LOCALIZE_SUCCESS)
else Error
SDK->>Tracking: trackEvent(LOCALIZE_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)
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.
Comment on lines
+78
to
+82
| headers: { | ||
| Authorization: `Bearer ${apiKey}`, | ||
| ContentType: "application/json", | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Fix HTTP header name typo: ContentType → Content-Type.
The header key ContentType is incorrect. HTTP headers use Content-Type (with a hyphen). This typo will cause the header to be sent with the wrong name, and the server won't recognize it as the content type header.
🐛 Proposed fix
headers: {
Authorization: `Bearer ${apiKey}`,
- ContentType: "application/json",
+ "Content-Type": "application/json",
},📝 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
| headers: { | |
| Authorization: `Bearer ${apiKey}`, | |
| ContentType: "application/json", | |
| }, | |
| }); | |
| headers: { | |
| Authorization: `Bearer ${apiKey}`, | |
| "Content-Type": "application/json", | |
| }, | |
| }); |
🤖 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 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.
|
|
||
| export const TRACKING_VERSION = "1.0"; | ||
|
|
||
| export const SDK_PACKAGE = "@lingo.dev/sdk"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/sdk/package.json | jq '.name'Repository: lingodotdev/lingo.dev
Length of output: 82
🏁 Script executed:
cat packages/sdk/src/utils/tracking-events.ts | head -20Repository: lingodotdev/lingo.dev
Length of output: 449
Correct SDK_PACKAGE to match the actual package name.
The SDK_PACKAGE constant is set to "@lingo.dev/sdk", but package.json defines the actual package name as "@lingo.dev/_sdk" (with underscore). Update line 12 to use the correct package name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/utils/tracking-events.ts` at 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.
AndreyHirsa
approved these changes
Feb 20, 2026
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
Chores