Closed
Conversation
added 2 commits
April 3, 2026 13:45
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR removes the Python backend and infra-gateway services and adds a new React + Vite frontend with an integrated Ghost telemetry SDK, PII masking, route/feature tracking, telemetry hooks, and dashboard UI components. Changes
Sequence Diagram(s)sequenceDiagram
participant React as React Component
participant Telemetry as GhostSDK
participant Mask as MaskingModule
participant Buffer as Buffer
participant API as API Service
participant Backend as Backend
React->>Telemetry: capture({eventType, featureModule, metadata})
Telemetry->>Telemetry: check consent & circuit breaker
alt consent denied
Telemetry->>React: return (dropped)
else consent granted
Telemetry->>Mask: maskEventPII(event)
Mask->>Mask: hash PII fields (async)
Mask->>Telemetry: return masked event
Telemetry->>Buffer: append masked event
Telemetry->>Telemetry: if buffer.size >= maxBufferSize then _flushBuffer()
alt flush triggered
Telemetry->>API: emitEvents(bufferedEvents)
API->>Backend: POST /api/v1/telemetry/events
Backend->>API: 200 OK
API->>Telemetry: success
Telemetry->>Buffer: clear
else
Telemetry->>React: return (queued)
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (4)
src/components/RiskModule.jsx-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorMissing component implementation.
This file contains only a comment with no React component definition or exports. If other parts of the application attempt to import this module, it will cause runtime errors.
⚛️ Scaffold a basic Risk Module component
-// Risk Module component +import React from 'react'; + +/** + * RiskModule component + * TODO: Implement risk analysis and display logic + */ +const RiskModule = () => { + return ( + <div className="risk-module"> + <h2>Risk Module</h2> + {/* TODO: Add risk module content */} + </div> + ); +}; + +export default RiskModule;Would you like me to open an issue to track the implementation of this component, or would you prefer to implement it as part of this PR?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RiskModule.jsx` at line 1, The file currently contains only a comment and must implement and export a React component named RiskModule so imports don't break; add an import React (or use react hooks import), define a functional component RiskModule (accepting props like className or data if needed), return a minimal JSX skeleton (e.g., container div with a header and placeholder content), and export it (export default RiskModule) so other modules can import it without runtime errors..gitignore-21-21 (1)
21-21:⚠️ Potential issue | 🟡 MinorConfiguration files should typically be committed.
Ignoring
vitest.config.jsprevents the test configuration from being shared across the team. Configuration files are normally version-controlled to ensure consistent development environments.If there are environment-specific overrides needed, consider:
- Committing
vitest.config.js(base config)- Ignoring
vitest.config.local.js(local overrides)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 21, Remove "vitest.config.js" from .gitignore so the shared test configuration is committed (ensure you add and commit the actual vitest.config.js file to the repo), and if you need local overrides add a new ignore entry such as "vitest.config.local.js" to .gitignore and document the override convention; reference the vitest.config.js filename in your change and update the repo accordingly.src/sdk/masking.js-115-118 (1)
115-118:⚠️ Potential issue | 🟡 Minor
maskTenantIdwill throw iftenantIdis null/empty.When
hashValuereturnsnullfor invalid input, calling.slice(0, 12)onnullthrows a TypeError. Add a guard or propagatenullgracefully.🛡️ Proposed fix
export async function maskTenantId(tenantId) { const hash = await hashValue(tenantId); + if (!hash) return null; return `tenant_${hash.slice(0, 12)}`; // Short prefix for readability in dashboards }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdk/masking.js` around lines 115 - 118, maskTenantId currently calls hashValue(tenantId) and then .slice(0,12) which will throw if tenantId is null/invalid and hashValue returns null; add input/result guards in maskTenantId: if tenantId is falsy return null (or propagate null) immediately, otherwise call hashValue and check its result before slicing—if hash is null/undefined return null (or propagate) instead of slicing; update maskTenantId to use the validated hash to build the `tenant_${...}` string only when safe.src/pages/Dashboard.jsx-382-383 (1)
382-383:⚠️ Potential issue | 🟡 MinorDrop the unused
lowcount.The status pills already compute their counts inline, so this binding is dead code and will keep failing
no-unused-vars.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Dashboard.jsx` around lines 382 - 383, The variable low declared as const low = ADOPTION_DATA.filter(d => d.status === "low").length is unused and triggers a no-unused-vars error; remove that binding from src/pages/Dashboard.jsx so only the needed zombies count remains (leave const zombies = ADOPTION_DATA.filter(d => d.status === "zombie").length) and rely on the inline status pill counts instead of creating the redundant low variable.
🧹 Nitpick comments (8)
.gitignore (1)
1-172: Consider consolidating into the existing organized structure.The file currently has two organizational styles:
- Lines 1-34: Unorganized frontend patterns
- Lines 35-172: Well-organized patterns with category headers and visual separators
For better maintainability, consider consolidating all patterns into the categorized structure. Unique patterns from lines 1-34 (like
dist-ssr,*.local) could be merged into the appropriate sections (e.g., "NODE / JAVASCRIPT").This would provide:
- Single source of truth for each pattern
- Easier navigation and updates
- Consistent style throughout
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 1 - 172, The top-of-file unorganized patterns (lines containing logs, *.log, node_modules, dist-ssr, *.local, .env entries) duplicate the later categorized sections; consolidate by removing the initial block and merging unique items like dist-ssr and *.local into the appropriate category headers (e.g., add dist-ssr and *.local under "NODE / JAVASCRIPT", move logs/*.log patterns under "LOGS", and ensure .env entries remain in "SECRETS & ENVIRONMENT"), eliminate duplicate lines so each pattern appears once, and keep the existing visual separators and category comments (use the "NODE / JAVASCRIPT", "LOGS", and "SECRETS & ENVIRONMENT" sections and symbols like dist-ssr, *.local, *.log, .env) to maintain the organized structure.eslint.config.js (2)
25-27:varsIgnorePatternmay be broader than intended.The pattern
^[A-Z_]matches any identifier starting with an uppercase letter or underscore (e.g.,Abc,_foo). If the intent is to ignore onlyUPPER_SNAKE_CASEconstants, consider^[A-Z_][A-Z0-9_]*$.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 25 - 27, The current no-unused-vars rule uses varsIgnorePattern '^[A-Z_]' which matches any identifier starting with an uppercase letter or underscore; update the pattern in the rules->'no-unused-vars' configuration to more precisely match UPPER_SNAKE_CASE constants (for example use a pattern like '^[A-Z_][A-Z0-9_]*$') so only true uppercase constants are ignored while other PascalCase or underscored identifiers remain checked.
16-23: RedundantecmaVersiondeclarations.
ecmaVersionis set to2020at thelanguageOptionslevel and'latest'inparserOptions. TheparserOptions.ecmaVersiontakes precedence, making the outer one redundant. Pick one for clarity.♻️ Proposed fix
languageOptions: { - ecmaVersion: 2020, globals: globals.browser, parserOptions: { ecmaVersion: 'latest',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 16 - 23, Remove the redundant ecmaVersion declaration by keeping a single authoritative setting: delete either languageOptions.ecmaVersion or parserOptions.ecmaVersion so only one remains; specifically update the eslint.config.js languageOptions block to use a single ecmaVersion setting (reference symbols: languageOptions, parserOptions, ecmaVersion) — prefer keeping parserOptions.ecmaVersion ('latest') and remove the outer languageOptions.ecmaVersion for clarity and to avoid confusion.src/main.jsx (1)
14-16: Hardcoded consent limits dynamic opt-out functionality.
CONSENT_GRANTEDis hardcoded totrue. As noted in the comment, this should be pulled from user settings. The current architecture (per context snippet) doesn't support propagating consent changes from Dashboard back to TelemetryProvider.For production, consider lifting consent state to the
main.jsxlevel with a state management solution, or makingTelemetryProviderinternally manage consent state that can be updated via context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.jsx` around lines 14 - 16, CONSENT_GRANTED is hardcoded in main.jsx which prevents dynamic opt-out; replace the constant by lifting consent into React state at the main.jsx level (useState) initialized from your auth/user settings instead of the literal true, pass the consent value and its setter into TelemetryProvider (or expose an updateConsent function from TelemetryProvider via context), and update Dashboard to consume the consent value and call the setter/updateConsent to toggle opt-in/out; keep TENANT_ID as-is but ensure TelemetryProvider respects the passed consent prop/state rather than reading a constant.src/sdk/masking.js (3)
42-51: Inconsistent input validation betweenhashValueandhashValueSync.
hashValuecheckstypeof value !== "string"and returnsnull, buthashValueSynconly checks for falsy values. This inconsistency could cause unexpected behavior when non-string values are passed.♻️ Proposed fix for consistent validation
export function hashValueSync(value) { - if (!value) return null; + if (!value || typeof value !== "string") return null; let hash = 0; for (let i = 0; i < value.length; i++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdk/masking.js` around lines 42 - 51, The synchronous function hashValueSync currently uses a falsy check which differs from hashValue's typeof check; change hashValueSync's input validation to mirror hashValue by replacing the `if (!value) return null;` check with `if (typeof value !== "string") return null;` so non-string inputs are consistently rejected (keep the rest of hashValueSync's hashing logic unchanged).
88-105: Shallow copy allows mutation of nested objects (exceptmetadata).The spread
{ ...eventObj }creates a shallow copy, so nested objects other thanmetadataare still references to the original. IfeventObjcontains nested arrays or objects with PII fields (beyondmetadata), those won't be masked and the original object's references may be inadvertently modified if future code mutatesmasked.Consider using a deep clone or documenting that only top-level and
metadataPII fields are masked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdk/masking.js` around lines 88 - 105, maskEventPII currently shallow-copies eventObj with const masked = { ...eventObj }, which leaves nested objects/arrays (other than metadata) as shared references so PII inside them won't be masked and original may be mutated; update maskEventPII to perform a deep clone of eventObj (or implement a recursive walk) before applying PII_FIELDS hashing so nested objects/arrays are traversed and masked, and ensure the recursion handles arrays and objects uniformly (including the existing metadata handling) to avoid mutating the original input; refer to maskEventPII, PII_FIELDS, and the masked variable when making the change.
129-143: Regex patterns may produce false positives.The Aadhaar pattern
\b\d{4}\s?\d{4}\s?\d{4}\bmatches any 12-digit number (e.g., transaction IDs, timestamps). The phone pattern could match within longer digit sequences. Consider tightening patterns or documenting the trade-off between recall and precision.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sdk/masking.js` around lines 129 - 143, The Aadhaar and phone regexes in redactPIIFromString are too permissive and can match parts of longer numeric strings; tighten them by adding digit lookarounds and stricter grouping: for Aadhaar update the pattern used in redactPIIFromString to require non-digit boundaries (e.g., use (?<!\d)(?:\d{4}\s?\d{4}\s?\d{4})(?!\d)) so only standalone 12-digit sequences (with optional spaces between 4-digit groups) match, and for Indian phones use digit lookarounds as well (e.g., (?<!\d)(?:\+91|0)?[6-9]\d{9}(?!\d)) to prevent matching inside longer numbers; keep the PAN and IP rules but consider ensuring PAN uses uppercase and word boundaries as already present. Ensure these updated regexes replace the existing Aadhaar and phone patterns inside redactPIIFromString.README.md (1)
1-16: README is generic template content.This is the default Vite template README. Consider customizing it to document the actual project (frontend-nexus), including the Ghost SDK telemetry integration, setup instructions, and environment variables like
VITE_TENANT_ID.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1 - 16, Replace the generic Vite template README with a project-specific README for "frontend-nexus": update the top-level description to explain frontend-nexus purpose and architecture, add a "Setup" section with exact steps to install dependencies and run dev/build scripts, add a "Ghost SDK telemetry" section describing how telemetry is integrated and any initialization steps, document required environment variables including VITE_TENANT_ID (purpose, example value, and where to place it, e.g., .env), and add brief sections for configuration, running tests, and contribution guidelines so maintainers can reproduce and configure the app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 1-34: Remove the duplicate leading ignore block (the added top
section) and rely on the existing organized ignore entries; specifically delete
the repeated patterns such as .env, node_modules, dist, logs, *.log,
npm-debug.log*, yarn-debug.log*, yarn-error.log*, .vscode/* and
!.vscode/extensions.json, .idea, .DS_Store and editor swap entries, and instead
merge any truly unique entries from the removed block (e.g., dist-ssr, *.local,
TEST_*.md, TESTING_*.md) into the appropriate categorized sections already
present in the file.
- Line 20: Remove the broad ignore entry "tests/" from .gitignore so test source
files are tracked; instead add only specific test artifact patterns you want
ignored (e.g., "tests/__pycache__/", "tests/.pytest_cache/", "tests/coverage/",
"tests/output/") and ensure test config files like "vitest.config.js" remain
committed.
In `@package.json`:
- Around line 26-27: Update the `@testing-library/react` dependency from "^15.0.0"
to "^16.1.0" in package.json to ensure React 19 compatibility; after changing
the version, run your package manager (npm/yarn/pnpm) to reinstall dependencies
and run the test suite to confirm there are no breaking changes, addressing any
peer-dependency or API adjustments flagged by tests or the installer.
- Around line 31-41: The project is using "vitest": "^1.2.2" which is
incompatible with Vite 8.0.1; update the vitest dependency in package.json (the
"vitest" entry) to a 4.x release (e.g., "^4.1.2"), run your package manager
install (npm/yarn/pnpm) to update lockfile, then run the test suite and fix any
Vitest 4.x breaking config changes (test/vitest config files) if they surface.
- Around line 11-18: The package.json contains test scripts (scripts "test",
"test:ui", "test:coverage", "test:masking", "test:taxonomy", "test:sdk",
"test:tracking", "test:all") that reference a non-existent tests/ directory and
missing test files and configuration; either create the tests/ folder and add
the referenced files (masking.test.js, feature-taxonomy.test.js,
ghost-sdk.test.js, tracking-wrapper.test.js) and add a Vitest config in
vite.config.js (or vitest.config.js) so vitest can run, or remove/disable the
non-functional scripts from package.json (remove or adjust the "test",
"test:masking", "test:taxonomy", "test:sdk", "test:tracking", and "test:all"
entries) so npm test and related commands no longer fail. Ensure the chosen fix
updates package.json scripts accordingly and, if adding tests, include basic
passing test stubs for the referenced filenames and a minimal vitest
configuration.
In `@src/components/EventPanel.jsx`:
- Line 1: The file currently only contains a comment and is missing the
EventPanel React component and export; implement a default-exported functional
component named EventPanel in this module, add a basic JSX skeleton (container
div, header, list placeholder), initialize Ghost SDK integration inside a
useEffect (call the SDK init and store client in state or ref), add simple
handlers (e.g., sendTelemetry, maskPII) that use the Ghost client for telemetry
and PII masking, and export default EventPanel so imports succeed; ensure hooks
(useState, useEffect, useRef) are imported from React and any Ghost SDK usage is
wrapped to avoid runtime errors if the SDK is unavailable.
In `@src/components/LoanModule.jsx`:
- Around line 27-37: The form currently overloads the step state in
LoanApplicationForm so handleStart sets step to 0 and the UI still treats 0 as
the "not started" state; change to use two separate states: keep numeric
progress state (step) and add a boolean lifecycle flag (e.g., isStarted or
journeyStarted). Update handleStart to check and set journeyStarted (and only
call journey.start() once), then initialize step to the first active index
(e.g., 1 or 0 depending on your step indexing) or enable progression; update
render checks that currently use step === 0 to instead use journeyStarted (or
the new flag) to show the Start button, and ensure handleNext increments the
numeric step only (and respects the lifecycle flag) so repeated clicks won’t
create multiple journeys.
- Around line 12-13: Update the import for the tracking wrapper to point to the
JSX module: change the import source used where withFeatureTracking and
wrapAPICall are imported so it references the tracking-wrapper.jsx file (the
same module used by TelemetryContext), ensuring the import line that brings in
withFeatureTracking and wrapAPICall matches the actual filename.
In `@src/context/TelemetryContext.jsx`:
- Around line 33-60: The effect must serialize async bootstrap/teardown to avoid
stale runs calling installRouteTracker, setSdkStatus, or GhostSDK.setConsent
after unmount or between tenant switches: modify the useEffect that calls async
function bootstrap and the cleanup to track a per-effect run token (or
AbortController) and await GhostSDK.init and GhostSDK.destroy; on cleanup cancel
the pending bootstrap (reject/ignore its results) and call/await
GhostSDK.destroy before returning, and guard post-init calls to
installRouteTracker() and setSdkStatus(GhostSDK.getStatus()) with the token (or
isMounted check) so they only run for the latest run; also ensure
GhostSDK.setConsent(consentGranted) is either delayed until after init or queued
when _config is missing (so use the same token/guard in the consent useEffect to
avoid dropped consent updates).
In `@src/hooks/useTelemetry.js`:
- Around line 58-69: useJourneyTelemetry currently stores journeyId in a local
variable so it resets on re-render; change it to persist the active id (e.g.,
useRef or useState) so start() writes the id to that persistent container and
step/complete/drop read from it. Update the function references
(useJourneyTelemetry, start, step, complete, drop, and sdk) to use the persisted
journeyId value instead of the local journeyId variable so re-renders don't null
out the id.
In `@src/pages/Dashboard.jsx`:
- Around line 562-623: The consent toggle and status rows are using local state
(consent, setConsent) and hardcoded values, so they don't update shared
telemetry state or call GhostSDK.setConsent; update the component to read and
update the shared telemetry context instead: import and use the context
values/hooks from src/context/TelemetryContext.jsx (e.g., consentGranted and
setConsentGranted or useTelemetry()), replace local consent/setConsent with the
context-provided consent state and setter, call GhostSDK.setConsent(...) (or the
context helper that wraps it) when toggling, and derive the "SDK Initialized"
and "Buffered Events"/"Circuit Breaker" row values from sdkStatus (already
present) rather than hardcoding "Yes" so the dashboard reflects the real runtime
state.
In `@src/sdk/ghost-sdk.js`:
- Around line 69-86: init() currently registers a new interval and an anonymous
beforeunload listener each time but destroy() only clears the interval, so
multiple unload handlers accumulate; make them symmetric by storing the
beforeunload handler reference (e.g., save a named function like
_beforeUnloadHandler or assign the anonymous to a variable) when init() adds
window.addEventListener("beforeunload", ...) and ensure destroy() both
clearInterval(_flushTimer) and call window.removeEventListener("beforeunload",
_beforeUnloadHandler) and reset/clear _flushTimer and the handler so re-init
doesn't leak.
- Around line 212-217: The telemetry calls to capture (e.g., the
EVENT_TYPE.JOURNEY_START call) are passing journeyName via featureModule which
fails FEATURE_MODULE validation; update those capture calls (lines using capture
with EVENT_TYPE.JOURNEY_START, JOURNEY_STEP, JOURNEY_COMPLETE, etc.) to set
featureModule to the proper enum value (FEATURE_MODULE.JOURNEY) and pass the
actual journey name only in metadata (e.g., metadata: { journeyName }) so
createEvent validates correctly and preserves the feature vs journey
distinction.
- Around line 297-310: In _flushBuffer, the catch can silently drop batchToSend
because new events may have filled _eventBuffer while emitEvents was pending; to
fix, when re-queuing in the catch of _flushBuffer (and honoring force), compute
the combined array = [...batchToSend, ..._eventBuffer] and then trim it to the
last _config.maxBufferSize items (or the first depending on intended order)
before assigning back to _eventBuffer, instead of checking _eventBuffer.length
alone; reference _flushBuffer, batchToSend, _eventBuffer, _config.maxBufferSize
and emitEvents and implement the requeue by merging and slicing to enforce the
cap.
In `@src/sdk/tracking-wrapper.jsx`:
- Around line 194-218: installRouteTracker currently adds a new popstate
listener and wraps history.pushState every time it's called, causing duplicate
tracking on re-renders; make it idempotent and disposable by tracking
installation state and stored originals: inside installRouteTracker check a
module-scoped flag (or property on window) to no-op if already installed, save
references to the added listener (trackRoute) and the original history.pushState
(origPushState) when installing, and return an uninstall function that removes
the popstate listener and restores history.pushState to origPushState and clears
the installed flag so subsequent installs behave correctly; update references to
installRouteTracker, trackRoute, popstate, history.pushState and origPushState
accordingly.
In `@src/services/api.js`:
- Around line 90-95: The updateTelemetryConsent function currently ignores the
HTTP response from fetch so callers can't detect when the PUT to
/governance/consent failed; modify updateTelemetryConsent to capture the fetch
Response, check response.ok and if false throw an Error (include status and any
response body/error message when available) so failures propagate to callers and
the UI/audit state stays consistent.
In `@vite.config.js`:
- Around line 5-7: The Vite config (export default defineConfig({ plugins:
[react()] })) missing Vitest settings causes component tests to run without a
DOM; update the defineConfig export in vite.config.js to include a test
configuration (e.g., add a test property) that sets environment: 'jsdom' (and
optionally globals: true and any setupFiles or setupFilesAfterEnv entries your
test suite needs) so `@testing-library/react` tests run in a jsdom environment.
---
Minor comments:
In @.gitignore:
- Line 21: Remove "vitest.config.js" from .gitignore so the shared test
configuration is committed (ensure you add and commit the actual
vitest.config.js file to the repo), and if you need local overrides add a new
ignore entry such as "vitest.config.local.js" to .gitignore and document the
override convention; reference the vitest.config.js filename in your change and
update the repo accordingly.
In `@src/components/RiskModule.jsx`:
- Line 1: The file currently contains only a comment and must implement and
export a React component named RiskModule so imports don't break; add an import
React (or use react hooks import), define a functional component RiskModule
(accepting props like className or data if needed), return a minimal JSX
skeleton (e.g., container div with a header and placeholder content), and export
it (export default RiskModule) so other modules can import it without runtime
errors.
In `@src/pages/Dashboard.jsx`:
- Around line 382-383: The variable low declared as const low =
ADOPTION_DATA.filter(d => d.status === "low").length is unused and triggers a
no-unused-vars error; remove that binding from src/pages/Dashboard.jsx so only
the needed zombies count remains (leave const zombies = ADOPTION_DATA.filter(d
=> d.status === "zombie").length) and rely on the inline status pill counts
instead of creating the redundant low variable.
In `@src/sdk/masking.js`:
- Around line 115-118: maskTenantId currently calls hashValue(tenantId) and then
.slice(0,12) which will throw if tenantId is null/invalid and hashValue returns
null; add input/result guards in maskTenantId: if tenantId is falsy return null
(or propagate null) immediately, otherwise call hashValue and check its result
before slicing—if hash is null/undefined return null (or propagate) instead of
slicing; update maskTenantId to use the validated hash to build the
`tenant_${...}` string only when safe.
---
Nitpick comments:
In @.gitignore:
- Around line 1-172: The top-of-file unorganized patterns (lines containing
logs, *.log, node_modules, dist-ssr, *.local, .env entries) duplicate the later
categorized sections; consolidate by removing the initial block and merging
unique items like dist-ssr and *.local into the appropriate category headers
(e.g., add dist-ssr and *.local under "NODE / JAVASCRIPT", move logs/*.log
patterns under "LOGS", and ensure .env entries remain in "SECRETS &
ENVIRONMENT"), eliminate duplicate lines so each pattern appears once, and keep
the existing visual separators and category comments (use the "NODE /
JAVASCRIPT", "LOGS", and "SECRETS & ENVIRONMENT" sections and symbols like
dist-ssr, *.local, *.log, .env) to maintain the organized structure.
In `@eslint.config.js`:
- Around line 25-27: The current no-unused-vars rule uses varsIgnorePattern
'^[A-Z_]' which matches any identifier starting with an uppercase letter or
underscore; update the pattern in the rules->'no-unused-vars' configuration to
more precisely match UPPER_SNAKE_CASE constants (for example use a pattern like
'^[A-Z_][A-Z0-9_]*$') so only true uppercase constants are ignored while other
PascalCase or underscored identifiers remain checked.
- Around line 16-23: Remove the redundant ecmaVersion declaration by keeping a
single authoritative setting: delete either languageOptions.ecmaVersion or
parserOptions.ecmaVersion so only one remains; specifically update the
eslint.config.js languageOptions block to use a single ecmaVersion setting
(reference symbols: languageOptions, parserOptions, ecmaVersion) — prefer
keeping parserOptions.ecmaVersion ('latest') and remove the outer
languageOptions.ecmaVersion for clarity and to avoid confusion.
In `@README.md`:
- Around line 1-16: Replace the generic Vite template README with a
project-specific README for "frontend-nexus": update the top-level description
to explain frontend-nexus purpose and architecture, add a "Setup" section with
exact steps to install dependencies and run dev/build scripts, add a "Ghost SDK
telemetry" section describing how telemetry is integrated and any initialization
steps, document required environment variables including VITE_TENANT_ID
(purpose, example value, and where to place it, e.g., .env), and add brief
sections for configuration, running tests, and contribution guidelines so
maintainers can reproduce and configure the app.
In `@src/main.jsx`:
- Around line 14-16: CONSENT_GRANTED is hardcoded in main.jsx which prevents
dynamic opt-out; replace the constant by lifting consent into React state at the
main.jsx level (useState) initialized from your auth/user settings instead of
the literal true, pass the consent value and its setter into TelemetryProvider
(or expose an updateConsent function from TelemetryProvider via context), and
update Dashboard to consume the consent value and call the setter/updateConsent
to toggle opt-in/out; keep TENANT_ID as-is but ensure TelemetryProvider respects
the passed consent prop/state rather than reading a constant.
In `@src/sdk/masking.js`:
- Around line 42-51: The synchronous function hashValueSync currently uses a
falsy check which differs from hashValue's typeof check; change hashValueSync's
input validation to mirror hashValue by replacing the `if (!value) return null;`
check with `if (typeof value !== "string") return null;` so non-string inputs
are consistently rejected (keep the rest of hashValueSync's hashing logic
unchanged).
- Around line 88-105: maskEventPII currently shallow-copies eventObj with const
masked = { ...eventObj }, which leaves nested objects/arrays (other than
metadata) as shared references so PII inside them won't be masked and original
may be mutated; update maskEventPII to perform a deep clone of eventObj (or
implement a recursive walk) before applying PII_FIELDS hashing so nested
objects/arrays are traversed and masked, and ensure the recursion handles arrays
and objects uniformly (including the existing metadata handling) to avoid
mutating the original input; refer to maskEventPII, PII_FIELDS, and the masked
variable when making the change.
- Around line 129-143: The Aadhaar and phone regexes in redactPIIFromString are
too permissive and can match parts of longer numeric strings; tighten them by
adding digit lookarounds and stricter grouping: for Aadhaar update the pattern
used in redactPIIFromString to require non-digit boundaries (e.g., use
(?<!\d)(?:\d{4}\s?\d{4}\s?\d{4})(?!\d)) so only standalone 12-digit sequences
(with optional spaces between 4-digit groups) match, and for Indian phones use
digit lookarounds as well (e.g., (?<!\d)(?:\+91|0)?[6-9]\d{9}(?!\d)) to prevent
matching inside longer numbers; keep the PAN and IP rules but consider ensuring
PAN uses uppercase and word boundaries as already present. Ensure these updated
regexes replace the existing Aadhaar and phone patterns inside
redactPIIFromString.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c7462c1-25d6-4e14-8aa8-b7c90c1e2ebb
⛔ Files ignored due to path filters (25)
.DS_Storeis excluded by!**/.DS_Storeai-brain/analytics/__pycache__/analytics_service.cpython-314.pycis excluded by!**/*.pycai-brain/analytics/__pycache__/benchmark.cpython-314.pycis excluded by!**/*.pycai-brain/analytics/__pycache__/funnel_analysis.cpython-314.pycis excluded by!**/*.pycai-brain/analytics/__pycache__/roi_calculator.cpython-314.pycis excluded by!**/*.pycai-brain/analytics/__pycache__/tenant_ranking.cpython-314.pycis excluded by!**/*.pycai-brain/analytics/__pycache__/zombie_detector.cpython-314.pycis excluded by!**/*.pycai-brain/api/__pycache__/app.cpython-314.pycis excluded by!**/*.pycai-brain/api/__pycache__/routes.cpython-314.pycis excluded by!**/*.pycai-brain/graph/__pycache__/feature_relationships.cpython-314.pycis excluded by!**/*.pycai-brain/graph/__pycache__/graph_builder.cpython-314.pycis excluded by!**/*.pycai-brain/graph/__pycache__/neo4j_client.cpython-314.pycis excluded by!**/*.pycai-brain/ingestion/__pycache__/event_consumer.cpython-314.pycis excluded by!**/*.pycai-brain/ingestion/__pycache__/mock_consumer.cpython-314.pycis excluded by!**/*.pycai-brain/ml_models/__pycache__/churn_predictor.cpython-314.pycis excluded by!**/*.pycai-brain/models/__pycache__/event_model.cpython-314.pycis excluded by!**/*.pycai-brain/processing/__pycache__/journey_engine.cpython-314.pycis excluded by!**/*.pycai-brain/processing/__pycache__/session_engine.cpython-314.pycis excluded by!**/*.pycai-brain/rag_advisor/__pycache__/advisor_bot.cpython-314.pycis excluded by!**/*.pycai-brain/simulation/__pycache__/journey_simulator.cpython-314.pycis excluded by!**/*.pycai-brain/utils/__pycache__/config.cpython-314.pycis excluded by!**/*.pycai-brain/utils/__pycache__/data_loader.cpython-314.pycis excluded by!**/*.pycai-brain/utils/__pycache__/logger.cpython-314.pycis excluded by!**/*.pycpublic/favicon.svgis excluded by!**/*.svgpublic/icons.svgis excluded by!**/*.svg
📒 Files selected for processing (81)
.gitignoreREADME.mdai-brain/analytics/analytics_service.pyai-brain/analytics/benchmark.pyai-brain/analytics/funnel_analysis.pyai-brain/analytics/roi_calculator.pyai-brain/analytics/tenant_ranking.pyai-brain/analytics/zombie_detector.pyai-brain/api/app.pyai-brain/api/routes.pyai-brain/graph/feature_relationships.pyai-brain/graph/graph_builder.pyai-brain/graph/neo4j_client.pyai-brain/ingestion/event_consumer.pyai-brain/ingestion/mock_consumer.pyai-brain/main.pyai-brain/ml_models/adoption_forecast.pyai-brain/ml_models/churn_predictor.pyai-brain/ml_models/clustering.pyai-brain/mock_data/config.jsonai-brain/mock_data/events.jsonai-brain/mock_data/features.jsonai-brain/mock_data/generator.pyai-brain/models/event_model.pyai-brain/models/feature_model.pyai-brain/processing/feature_engineering.pyai-brain/processing/journey_engine.pyai-brain/processing/session_engine.pyai-brain/rag_advisor/advisor_bot.pyai-brain/rag_advisor/vector_store.pyai-brain/requirements.txtai-brain/simulation/journey_simulator.pyai-brain/utils/config.pyai-brain/utils/data_loader.pyai-brain/utils/logger.pycollector-sdk/package.jsoncollector-sdk/src/interceptors.jscollector-sdk/src/logger.jscollector-sdk/src/masking.jscontracts/event_schema.jsoneslint.config.jsindex.htmlinfra-gateway/cloud_ingestor/Dockerfile.ingestorinfra-gateway/cloud_ingestor/__init__.pyinfra-gateway/cloud_ingestor/kafka_config.pyinfra-gateway/cloud_ingestor/requirements.txtinfra-gateway/docker-compose.ymlinfra-gateway/on_prem_vault/Dockerfile.aggregatorinfra-gateway/on_prem_vault/Dockerfile.syncinfra-gateway/on_prem_vault/__init__.pyinfra-gateway/on_prem_vault/aggregator.pyinfra-gateway/on_prem_vault/requirements.txtinfra-gateway/on_prem_vault/sync_service.pyinfra-gateway/pytest.iniinfra-gateway/render.yamlinfra-gateway/requirements.txtinfra-gateway/security/rbac_config.jsoninfra-gateway/test.jsoninfra-gateway/tests/conftest.pyinfra-gateway/tests/test_person_b.pypackage.jsonsrc/App.jsxsrc/components/DocumentModule.jsxsrc/components/EventPanel.jsxsrc/components/LoanModule.jsxsrc/components/RiskModule.jsxsrc/context/TelemetryContext.jsxsrc/hooks/useTelemetry.jssrc/main.jsxsrc/pages/Dashboard.jsxsrc/sdk/feature-taxonomy.jssrc/sdk/ghost-sdk.jssrc/sdk/masking.jssrc/sdk/tracking-wrapper.jsxsrc/services/api.jsvite.config.jsweb-dashboard/package.jsonweb-dashboard/src/components/AIInsights.jsxweb-dashboard/src/components/Funnels.jsxweb-dashboard/src/components/Heatmaps.jsxweb-dashboard/src/{App.js}
💤 Files with no reviewable changes (44)
- ai-brain/mock_data/events.json
- ai-brain/mock_data/features.json
- ai-brain/utils/logger.py
- infra-gateway/test.json
- ai-brain/api/app.py
- ai-brain/analytics/benchmark.py
- infra-gateway/tests/conftest.py
- ai-brain/ingestion/mock_consumer.py
- infra-gateway/cloud_ingestor/requirements.txt
- ai-brain/ingestion/event_consumer.py
- ai-brain/analytics/zombie_detector.py
- ai-brain/mock_data/config.json
- ai-brain/utils/data_loader.py
- infra-gateway/pytest.ini
- infra-gateway/cloud_ingestor/Dockerfile.ingestor
- ai-brain/graph/neo4j_client.py
- infra-gateway/render.yaml
- ai-brain/analytics/roi_calculator.py
- infra-gateway/requirements.txt
- ai-brain/analytics/funnel_analysis.py
- ai-brain/models/feature_model.py
- ai-brain/processing/session_engine.py
- ai-brain/analytics/tenant_ranking.py
- infra-gateway/on_prem_vault/requirements.txt
- ai-brain/models/event_model.py
- ai-brain/graph/feature_relationships.py
- infra-gateway/on_prem_vault/Dockerfile.aggregator
- contracts/event_schema.json
- ai-brain/graph/graph_builder.py
- infra-gateway/on_prem_vault/Dockerfile.sync
- ai-brain/analytics/analytics_service.py
- ai-brain/rag_advisor/advisor_bot.py
- ai-brain/main.py
- infra-gateway/security/rbac_config.json
- ai-brain/simulation/journey_simulator.py
- infra-gateway/docker-compose.yml
- ai-brain/api/routes.py
- ai-brain/ml_models/churn_predictor.py
- infra-gateway/on_prem_vault/aggregator.py
- infra-gateway/tests/test_person_b.py
- ai-brain/processing/journey_engine.py
- infra-gateway/on_prem_vault/sync_service.py
- ai-brain/utils/config.py
- infra-gateway/cloud_ingestor/kafka_config.py
Comment on lines
+1
to
+34
| # Logs | ||
| logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
| pnpm-debug.log* | ||
| lerna-debug.log* | ||
|
|
||
| node_modules | ||
| dist | ||
| dist-ssr | ||
| *.local | ||
|
|
||
| # Environment variables | ||
| .env | ||
| .env.local | ||
|
|
||
| # Tests | ||
| tests/ | ||
| vitest.config.js | ||
| TEST_*.md | ||
| TESTING_*.md | ||
|
|
||
| # Editor directories and files | ||
| .vscode/* | ||
| !.vscode/extensions.json | ||
| .idea | ||
| .DS_Store | ||
| *.suo | ||
| *.ntvs* | ||
| *.njsproj | ||
| *.sln | ||
| *.sw? |
There was a problem hiding this comment.
Remove duplicate ignore patterns.
The newly added section (lines 1-34) contains numerous patterns that already exist in the organized section below (lines 35-172):
.env(line 16 duplicates lines 43-44)node_modules(line 10 duplicates line 108)dist(line 11 duplicates lines 70, 117)logs(line 2 duplicates line 160)*.log(line 3 duplicates line 159)npm-debug.log*,yarn-debug.log*,yarn-error.log*(lines 4-6 duplicate lines 109-111).vscode/*/!.vscode/extensions.json(lines 26-27 duplicate lines 137-138).idea(line 28 duplicates line 140).DS_Store(line 29 duplicates line 149)- Editor swap files (line 34 duplicates lines 144-145)
While duplicate patterns don't break functionality, they create maintenance burden and confusion.
🧹 Recommended cleanup
Remove the duplicate section (lines 1-34) and rely on the existing well-organized patterns (lines 35-172). If any patterns in lines 1-34 are truly unique (e.g., dist-ssr, *.local, TEST_*.md, TESTING_*.md), merge them into the appropriate categorized sections below.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore around lines 1 - 34, Remove the duplicate leading ignore block
(the added top section) and rely on the existing organized ignore entries;
specifically delete the repeated patterns such as .env, node_modules, dist,
logs, *.log, npm-debug.log*, yarn-debug.log*, yarn-error.log*, .vscode/* and
!.vscode/extensions.json, .idea, .DS_Store and editor swap entries, and instead
merge any truly unique entries from the removed block (e.g., dist-ssr, *.local,
TEST_*.md, TESTING_*.md) into the appropriate categorized sections already
present in the file.
| .env.local | ||
|
|
||
| # Tests | ||
| tests/ |
There was a problem hiding this comment.
Ignoring the entire tests/ directory prevents committing test files.
Line 20 ignores the entire tests/ directory. This pattern will exclude all test source files from version control, which is almost certainly incorrect. Test code should be committed to the repository.
If the intent is to ignore test artifacts or coverage reports, use more specific patterns like:
tests/__pycache__/(already covered by line 61)tests/.pytest_cache/(already covered by line 76)tests/coverage/tests/output/
🧪 Recommended fix
-# Tests
-tests/
-vitest.config.js
-TEST_*.md
-TESTING_*.md
+# Test artifacts (not test source files)
+tests/coverage/
+tests/output/
+TEST_*.md
+TESTING_*.mdNote: vitest.config.js should also be committed as it's a configuration file (see separate comment).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 20, Remove the broad ignore entry "tests/" from
.gitignore so test source files are tracked; instead add only specific test
artifact patterns you want ignored (e.g., "tests/__pycache__/",
"tests/.pytest_cache/", "tests/coverage/", "tests/output/") and ensure test
config files like "vitest.config.js" remain committed.
Comment on lines
+11
to
+18
| "test": "vitest", | ||
| "test:ui": "vitest --ui", | ||
| "test:coverage": "vitest --coverage", | ||
| "test:masking": "vitest tests/masking.test.js", | ||
| "test:taxonomy": "vitest tests/feature-taxonomy.test.js", | ||
| "test:sdk": "vitest tests/ghost-sdk.test.js", | ||
| "test:tracking": "vitest tests/tracking-wrapper.test.js", | ||
| "test:all": "vitest tests/ --run" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if tests directory and test files exist
fd -t d 'tests'
fd -t f '\.test\.' --extension js --extension jsx --extension tsRepository: meharkp7/NEXUS
Length of output: 40
🏁 Script executed:
git ls-files | grep -i test | head -20Repository: meharkp7/NEXUS
Length of output: 40
🏁 Script executed:
fd -t f -i 'test' . | head -20Repository: meharkp7/NEXUS
Length of output: 40
🏁 Script executed:
# Look for any vitest or test configuration
fd -e config.js -e vitest.config.js -e jest.config.jsRepository: meharkp7/NEXUS
Length of output: 89
🏁 Script executed:
# View overall repo structure
ls -laRepository: meharkp7/NEXUS
Length of output: 1050
🏁 Script executed:
# Check vite.config.js for test configuration
cat vite.config.jsRepository: meharkp7/NEXUS
Length of output: 218
🏁 Script executed:
# Check if package.json has any other test-related setup
cat package.json | head -30Repository: meharkp7/NEXUS
Length of output: 947
🏁 Script executed:
cat package.json | grep -A 50 "devDependencies"Repository: meharkp7/NEXUS
Length of output: 629
Test scripts reference non-existent test files and missing infrastructure.
The tests/ directory does not exist, and none of the referenced test files (masking.test.js, feature-taxonomy.test.js, ghost-sdk.test.js, tracking-wrapper.test.js) are present in the repository. While vitest is installed as a dependency, there is no vitest configuration in vite.config.js, and running any test script (e.g., npm test) will fail. Either create the test directory and implement the referenced test files, or remove the non-functional test scripts from package.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 11 - 18, The package.json contains test scripts
(scripts "test", "test:ui", "test:coverage", "test:masking", "test:taxonomy",
"test:sdk", "test:tracking", "test:all") that reference a non-existent tests/
directory and missing test files and configuration; either create the tests/
folder and add the referenced files (masking.test.js, feature-taxonomy.test.js,
ghost-sdk.test.js, tracking-wrapper.test.js) and add a Vitest config in
vite.config.js (or vitest.config.js) so vitest can run, or remove/disable the
non-functional scripts from package.json (remove or adjust the "test",
"test:masking", "test:taxonomy", "test:sdk", "test:tracking", and "test:all"
entries) so npm test and related commands no longer fail. Ensure the chosen fix
updates package.json scripts accordingly and, if adding tests, include basic
passing test stubs for the referenced filenames and a minimal vitest
configuration.
Comment on lines
+26
to
+27
| "@testing-library/react": "^15.0.0", | ||
| "@testing-library/user-event": "^14.5.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What version of @testing-library/react supports React 19?
💡 Result:
Version 16.1.0 and later of @testing-library/react support React 19. React 19 support was explicitly added in v16.1.0, as confirmed by maintainers in GitHub issues where users resolved React 19 peer dependency conflicts by upgrading to ^16.1.0. Later versions like v16.3.2 (latest as of 2026-01-19) continue this support, including React 19-specific fixes such as type inference for error handlers. The project's dev dependencies also include react@^19.0.0, confirming compatibility.
Citations:
- 1: React v19 testing-library/react-testing-library#1373
- 2: Components do not fully render anymore with React 19 testing-library/react-testing-library#1397
- 3: Support for React 19 testing-library/react-testing-library#1364
- 4: https://github.com/testing-library/react-testing-library/releases
- 5: https://www.npmjs.com/package/@testing-library/react?activeTab=versions
- 6: https://github.com/testing-library/react-testing-library/releases/tag/v16.2.0
- 7: https://newreleases.io/project/github/testing-library/react-testing-library/release/v16.3.2
- 8: https://www.myreleasenotes.ai/packages/testing-library/react-testing-library
Upgrade @testing-library/react to v16.1.0 or later for React 19 support.
@testing-library/react@15.0.0 is incompatible with React 19. React 19 support requires v16.1.0+. Update to ^16.1.0 to resolve potential runtime errors and testing failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 26 - 27, Update the `@testing-library/react`
dependency from "^15.0.0" to "^16.1.0" in package.json to ensure React 19
compatibility; after changing the version, run your package manager
(npm/yarn/pnpm) to reinstall dependencies and run the test suite to confirm
there are no breaking changes, addressing any peer-dependency or API adjustments
flagged by tests or the installer.
Comment on lines
+31
to
+41
| "@vitest/ui": "^1.2.2", | ||
| "autoprefixer": "^10.4.27", | ||
| "eslint": "^9.39.4", | ||
| "eslint-plugin-react-hooks": "^7.0.1", | ||
| "eslint-plugin-react-refresh": "^0.5.2", | ||
| "globals": "^17.4.0", | ||
| "jsdom": "^24.0.0", | ||
| "postcss": "^8.5.8", | ||
| "tailwindcss": "^4.2.2", | ||
| "vite": "^8.0.1", | ||
| "vitest": "^1.2.2" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What version of Vitest is compatible with Vite 8?
💡 Result:
Vitest 4.x (latest version 4.1.2 as of March 2026) is compatible with Vite 8. Vitest lists Vite ^6.0.0 || ^7.0.0 || ^8.0.0 as both a dependency and peer dependency, explicitly supporting Vite 8. Vitest 4.0 requires Vite >=6.0.0, and recent updates (e.g., Vitest 4.1 betas and releases) include support for Vite 8, with issues and PRs confirming usage with Vite 8 (e.g., dropping beta support after Vite 8 stable release on 2026-03-12).
Citations:
- 1: https://www.npmjs.com/package/vitest?activeTab=dependencies
- 2: https://vitest.dev/guide/migration.html
- 3: https://v4.vitest.dev/guide/migration
- 4: https://github.com/vitest-dev/vitest/releases
- 5: Vitest 4 & Vite 8 - Consistent CommonJS Interop vitest-dev/vitest#9922
- 6: fix: drop vite 8.beta support vitest-dev/vitest#9862
- 7: vitest-dev/vitest@36f9a81a2
Upgrade Vitest to 4.x for Vite 8.0.1 compatibility.
Vitest 1.2.2 is incompatible with Vite 8.0.1. Vitest 1.x was built for Vite 5.x and earlier. Vitest 4.x (currently 4.1.2) explicitly supports Vite 8 via the peer dependency ^8.0.0. Upgrade to Vitest 4.x.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 31 - 41, The project is using "vitest": "^1.2.2"
which is incompatible with Vite 8.0.1; update the vitest dependency in
package.json (the "vitest" entry) to a 4.x release (e.g., "^4.1.2"), run your
package manager install (npm/yarn/pnpm) to update lockfile, then run the test
suite and fix any Vitest 4.x breaking config changes (test/vitest config files)
if they surface.
Comment on lines
+212
to
+217
| await capture({ | ||
| eventType: EVENT_TYPE.JOURNEY_START, | ||
| featureModule: journeyName, | ||
| journeyId, | ||
| metadata: { journeyName }, | ||
| }); |
There was a problem hiding this comment.
Journey telemetry is being written into the wrong field.
These methods pass journeyName through featureModule. createEvent() validates featureModule against FEATURE_MODULE, so normal journey events are emitted as “unknown featureModule”, and downstream consumers lose the feature-vs-journey distinction.
Also applies to: 237-243, 256-260, 277-282
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/ghost-sdk.js` around lines 212 - 217, The telemetry calls to capture
(e.g., the EVENT_TYPE.JOURNEY_START call) are passing journeyName via
featureModule which fails FEATURE_MODULE validation; update those capture calls
(lines using capture with EVENT_TYPE.JOURNEY_START, JOURNEY_STEP,
JOURNEY_COMPLETE, etc.) to set featureModule to the proper enum value
(FEATURE_MODULE.JOURNEY) and pass the actual journey name only in metadata
(e.g., metadata: { journeyName }) so createEvent validates correctly and
preserves the feature vs journey distinction.
Comment on lines
+297
to
+310
| async function _flushBuffer(force = false) { | ||
| if (_eventBuffer.length === 0) return; | ||
|
|
||
| const batchToSend = [..._eventBuffer]; | ||
| _eventBuffer = []; // Clear buffer immediately to avoid duplicate sends | ||
|
|
||
| try { | ||
| await emitEvents(batchToSend); | ||
| if (_config?.debug) console.log(`[GhostSDK] Flushed ${batchToSend.length} events`); | ||
| } catch (err) { | ||
| // On failure, re-queue events (up to max buffer size to avoid memory leak) | ||
| if (!force && _eventBuffer.length < _config.maxBufferSize) { | ||
| _eventBuffer = [...batchToSend, ..._eventBuffer]; | ||
| } |
There was a problem hiding this comment.
The retry guard can discard the batch that just failed.
If new events arrive while emitEvents(batchToSend) is pending, _eventBuffer.length may already be >= maxBufferSize by the time the catch runs, so batchToSend never gets requeued. That's silent telemetry loss on the failure path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/ghost-sdk.js` around lines 297 - 310, In _flushBuffer, the catch can
silently drop batchToSend because new events may have filled _eventBuffer while
emitEvents was pending; to fix, when re-queuing in the catch of _flushBuffer
(and honoring force), compute the combined array = [...batchToSend,
..._eventBuffer] and then trim it to the last _config.maxBufferSize items (or
the first depending on intended order) before assigning back to _eventBuffer,
instead of checking _eventBuffer.length alone; reference _flushBuffer,
batchToSend, _eventBuffer, _config.maxBufferSize and emitEvents and implement
the requeue by merging and slicing to enforce the cap.
Comment on lines
+194
to
+218
| export function installRouteTracker() { | ||
| const trackRoute = () => { | ||
| const feature = resolveFeatureFromRoute(window.location.pathname); | ||
| if (feature) { | ||
| GhostSDK.capture({ | ||
| eventType: EVENT_TYPE.FEATURE_OPEN, | ||
| featureModule: feature, | ||
| metadata: { route: window.location.pathname }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| // Intercept browser back/forward | ||
| window.addEventListener("popstate", trackRoute); | ||
|
|
||
| // Intercept React Router programmatic navigation (pushState) | ||
| const origPushState = history.pushState.bind(history); | ||
| history.pushState = function (...args) { | ||
| origPushState(...args); | ||
| trackRoute(); | ||
| }; | ||
|
|
||
| // Track initial page load | ||
| trackRoute(); | ||
| } |
There was a problem hiding this comment.
Make route tracker installation idempotent and disposable.
Every call adds another popstate listener and wraps history.pushState again, but there is no cleanup path. Since TelemetryContext installs this after SDK bootstrap, tenant changes or StrictMode replays will double-count page opens.
Patch sketch
+let routeTrackerCleanup = null;
+
export function installRouteTracker() {
+ if (routeTrackerCleanup) return routeTrackerCleanup;
+
const trackRoute = () => {
const feature = resolveFeatureFromRoute(window.location.pathname);
if (feature) {
@@
};
// Intercept browser back/forward
- window.addEventListener("popstate", trackRoute);
+ const handlePopState = () => trackRoute();
+ window.addEventListener("popstate", handlePopState);
// Intercept React Router programmatic navigation (pushState)
const origPushState = history.pushState.bind(history);
history.pushState = function (...args) {
origPushState(...args);
trackRoute();
};
// Track initial page load
trackRoute();
+
+ routeTrackerCleanup = () => {
+ window.removeEventListener("popstate", handlePopState);
+ history.pushState = origPushState;
+ routeTrackerCleanup = null;
+ };
+
+ return routeTrackerCleanup;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sdk/tracking-wrapper.jsx` around lines 194 - 218, installRouteTracker
currently adds a new popstate listener and wraps history.pushState every time
it's called, causing duplicate tracking on re-renders; make it idempotent and
disposable by tracking installation state and stored originals: inside
installRouteTracker check a module-scoped flag (or property on window) to no-op
if already installed, save references to the added listener (trackRoute) and the
original history.pushState (origPushState) when installing, and return an
uninstall function that removes the popstate listener and restores
history.pushState to origPushState and clears the installed flag so subsequent
installs behave correctly; update references to installRouteTracker, trackRoute,
popstate, history.pushState and origPushState accordingly.
Comment on lines
+90
to
+95
| export async function updateTelemetryConsent(maskedTenantId, consentGranted) { | ||
| await fetch(`${BASE_URL}/api/v1/governance/consent`, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ tenantId: maskedTenantId, consentGranted }), | ||
| }); |
There was a problem hiding this comment.
Check the consent endpoint response before returning success.
fetch() resolves on HTTP errors, so a rejected /governance/consent write currently looks identical to a successful one. Callers need an exception here or the UI/audit state will diverge.
Patch sketch
export async function updateTelemetryConsent(maskedTenantId, consentGranted) {
- await fetch(`${BASE_URL}/api/v1/governance/consent`, {
+ const response = await fetch(`${BASE_URL}/api/v1/governance/consent`, {
method: "PUT",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ tenantId: maskedTenantId, consentGranted }),
});
+
+ if (!response.ok) {
+ throw new Error(`[InsightOS API] Consent update failed: ${response.status}`);
+ }
}📝 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
| export async function updateTelemetryConsent(maskedTenantId, consentGranted) { | |
| await fetch(`${BASE_URL}/api/v1/governance/consent`, { | |
| method: "PUT", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ tenantId: maskedTenantId, consentGranted }), | |
| }); | |
| export async function updateTelemetryConsent(maskedTenantId, consentGranted) { | |
| const response = await fetch(`${BASE_URL}/api/v1/governance/consent`, { | |
| method: "PUT", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ tenantId: maskedTenantId, consentGranted }), | |
| }); | |
| if (!response.ok) { | |
| throw new Error(`[InsightOS API] Consent update failed: ${response.status}`); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/api.js` around lines 90 - 95, The updateTelemetryConsent
function currently ignores the HTTP response from fetch so callers can't detect
when the PUT to /governance/consent failed; modify updateTelemetryConsent to
capture the fetch Response, check response.ok and if false throw an Error
(include status and any response body/error message when available) so failures
propagate to callers and the UI/audit state stays consistent.
Comment on lines
+5
to
+7
| export default defineConfig({ | ||
| plugins: [react()], | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any vitest.config.js or separate test config exists
fd -t f -i 'vitest' --extension js --extension ts --extension mjs
# Check if there's a setupTests file
fd -i 'setup' -e js -e ts | head -20Repository: meharkp7/NEXUS
Length of output: 40
🏁 Script executed:
# Read the full vite.config.js file
cat vite.config.js
# Check package.json for jsdom and test scripts
cat package.json | head -50Repository: meharkp7/NEXUS
Length of output: 1436
🏁 Script executed:
# Check if test configuration might be in vitest config or elsewhere
fd -t f 'vitest' -e config -e js -e ts
# Check what's in the tests directory
fd -t f 'test' -type f | head -20
# Check if there's any setupFiles configuration elsewhere
rg 'setupFiles|environment.*jsdom' --type js --type tsRepository: meharkp7/NEXUS
Length of output: 228
🏁 Script executed:
# Check if test configuration might be in vitest config
fd 'vitest' -e config -e js -e ts
# Check what's in the tests directory
find . -type f -name "*test*" | head -20
# Check if there's any setupFiles configuration or jsdom environment elsewhere
rg 'setupFiles|environment.*jsdom'Repository: meharkp7/NEXUS
Length of output: 1743
🏁 Script executed:
# Check for test files in the project root and src, excluding node_modules
find . -path ./node_modules -prune -o -type f -name "*test*" -print | head -20
# Check for vitest.config in project root
ls -la | grep -i vitest
# Search for jsdom or environment configuration outside node_modules
rg 'setupFiles|environment.*jsdom' --glob '!node_modules'Repository: meharkp7/NEXUS
Length of output: 40
🏁 Script executed:
# List all files in the repo (excluding node_modules)
git ls-files | grep -i test
# Check the root directory
ls -la vite* vitest*
# Check if there's any test configuration in vite.config.js already
cat vite.config.jsRepository: meharkp7/NEXUS
Length of output: 353
Add Vitest configuration to vite.config.js for jsdom environment.
package.json declares jsdom and vitest as devDependencies and includes test scripts referencing test files, but vite.config.js lacks the test configuration. React component tests using @testing-library/react require a DOM environment.
🔧 Proposed fix
import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'
// https://vite.dev/config/
export default defineConfig({
plugins: [react()],
+ test: {
+ environment: 'jsdom',
+ globals: true,
+ },
})📝 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
| export default defineConfig({ | |
| plugins: [react()], | |
| }) | |
| import { defineConfig } from 'vite' | |
| import react from '@vitejs/plugin-react' | |
| // https://vite.dev/config/ | |
| export default defineConfig({ | |
| plugins: [react()], | |
| test: { | |
| environment: 'jsdom', | |
| globals: true, | |
| }, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vite.config.js` around lines 5 - 7, The Vite config (export default
defineConfig({ plugins: [react()] })) missing Vitest settings causes component
tests to run without a DOM; update the defineConfig export in vite.config.js to
include a test configuration (e.g., add a test property) that sets environment:
'jsdom' (and optionally globals: true and any setupFiles or setupFilesAfterEnv
entries your test suite needs) so `@testing-library/react` tests run in a jsdom
environment.
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
1 participant
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 by CodeRabbit
Removed Features
New Features
Documentation
Chores
.gitignorewith Node.js, environment, and IDE patterns.