feat: implement SaaS auth gating with tiered access model - #197
Conversation
✅ Deploy Preview for dev-bayanflow ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThis PR adds tiered entitlement gating across algorithms, controls, visualizers, and export flows, introduces theme-switch audio playback, and updates supporting translations, auth/session handling, docs, and tests. It also includes a few unrelated UI/config adjustments. ChangesTiered entitlement and feature gating
Theme switch sound effect
Unrelated UI and config updates
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Visualizer
participant entitlementService
participant ComplexityPanel
Visualizer->>entitlementService: canViewComplexityPanel(user)
alt allowed
Visualizer->>entitlementService: incrementComplexityViewCount()
Visualizer->>ComplexityPanel: render panel
else blocked
Visualizer->>Visualizer: setIsComplexityGated(true)
Visualizer->>Visualizer: onGatedFeatureClick('complexity_limit')
Visualizer->>ComplexityPanel: render blurred overlay
end
sequenceDiagram
participant User
participant VisualizerApp
participant entitlementService
participant useVideoExporter
User->>VisualizerApp: handleExportVideo()
VisualizerApp->>VisualizerApp: openGatedFeature('export') if unauthenticated
VisualizerApp->>entitlementService: canRunVideoExport(user)
alt allowed
VisualizerApp->>entitlementService: incrementVideoExportCount(user)
VisualizerApp->>useVideoExporter: exportVideo(watermark)
else blocked
VisualizerApp->>useVideoExporter: reportExportError(message)
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
Preview for Bayan Flow Staging ready!
Preview alias |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AGENTS.md (1)
13-43: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep
AGENTS.mdcontract-only.The snapshot counts/routes/test-command catalog are volatile reference data, not non-negotiable rules. Leaving them here will churn this file on every registry change.
As per coding guidelines, update
docs/AGENTS_REFERENCE.mdonly for counts, file paths, and test commands.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` around lines 13 - 43, Keep AGENTS.md contract-only by removing the volatile snapshot data from this section: the tooling/version counts, route table, source/test counts, and source-of-truth registry listings should not live here. Move those reference details to docs/AGENTS_REFERENCE.md, and keep AGENTS.md limited to stable rules and workflow guidance so it doesn’t churn when registries or counts change.Source: Coding guidelines
🧹 Nitpick comments (8)
src/components/ArrayVisualizer.jsx (1)
106-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider extracting the complexity-gate logic into a shared hook.
This exact block — the three effects (
reset on isComplete,clear gate on user,count-once-per-completion) plus thehasCountedThisCompletionref andisComplexityGatedstate — is duplicated verbatim inGraphAlgorithmMatrixVisualizer.jsx,GridVisualizer.jsx, andTreeVisualizer.jsx(and the overlay markup at Lines 175-188 is likewise copy-pasted). The copies are already drifting slightly (e.g., TreeVisualizer drops the explicitelse). A singleuseComplexityGate(user, isComplete, onGatedFeatureClick)hook returningisComplexityGated, paired with a smallComplexityGateOverlaycomponent, would centralize this and prevent divergence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ArrayVisualizer.jsx` around lines 106 - 123, Extract the duplicated complexity-gate behavior from ArrayVisualizer’s effect block into a shared useComplexityGate(user, isComplete, onGatedFeatureClick) hook, keeping the reset-on-completion, clear-on-user, and count-once-per-completion logic in one place. Have the hook own the hasCountedThisCompletion ref and isComplexityGated state, and update GraphAlgorithmMatrixVisualizer, GridVisualizer, and TreeVisualizer to consume it instead of inlining the same effects. Also pull the repeated overlay markup into a small ComplexityGateOverlay component so all visualizers render the gate UI consistently and stop drifting.src/services/entitlementService.js (2)
215-237: 🧹 Nitpick | 🔵 TrivialFree-tier daily export cap is purely client-side and trivially bypassable.
canRunVideoExport/readDailyExportCount/writeDailyExportCountenforce the 50/day free-tier export limit entirely vialocalStoragekeyed by a locally-hashed identifier. Any user can bypass this by clearing site storage, using a private window, or switching browsers — no server-side accounting exists. Same applies to the anonymous visualization/complexity-view counters (Lines 147-208). Given the PR frames this as an explicit product limit ("50 exports per day"), consider whether server-side enforcement (or at least server-side accounting used to reconcile) is planned, since the current guard only deters casual overuse rather than enforcing the stated limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/entitlementService.js` around lines 215 - 237, The free-tier export cap is only enforced in client-side helpers, so it can be bypassed by clearing storage or changing browsers; update the entitlement flow around canRunVideoExport, readDailyExportCount, writeDailyExportCount, and incrementVideoExportCount so the 50/day limit is backed by server-side accounting or validation rather than localStorage alone. Apply the same fix approach to the anonymous counter logic in the related visualization/complexity-view helpers so the product limit is enforced consistently across sessions and devices.
49-76: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInconsistent error handling around
localStorageaccess.
readDailyExportCountwraps its localStorage/JSON access in try/catch, butwriteDailyExportCount(and later,incrementVisualizationCount,incrementComplexityViewCount,canRunVisualization,getRemainingVisualizations,canViewComplexityPanel) calllocalStorage.getItem/setItemdirectly without guarding. IflocalStorageis unavailable or throws (quota exceeded, restricted storage policies), these unguarded calls will throw and break the calling flow (e.g.handlePlay, video export) for anonymous/free users.Consider a small shared safe-storage helper reused across all these functions.
🔧 Proposed helper
+function safeLocalStorageSet(key, value) { + try { + localStorage.setItem(key, value); + } catch { + // Ignore storage failures; feature degrades gracefully. + } +} + function writeDailyExportCount(user, count) { const storageKey = getDailyExportStorageKey(user); if (!storageKey) return; - localStorage.setItem( + safeLocalStorageSet( storageKey, JSON.stringify({ date: getUtcDateKey(), count }) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/entitlementService.js` around lines 49 - 76, `readDailyExportCount` already defensively handles storage failures, but `writeDailyExportCount` and the other entitlement helpers still access `localStorage` directly and can throw in restricted environments. Add a small shared safe-storage wrapper around `localStorage.getItem`/`setItem` and use it from `writeDailyExportCount`, `incrementVisualizationCount`, `incrementComplexityViewCount`, `canRunVisualization`, `getRemainingVisualizations`, and `canViewComplexityPanel` so the entitlement flow keeps working when storage is unavailable or quota-limited.src/components/AlgorithmDropdown.test.jsx (1)
112-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage of the locked-click path. Consider also adding a case with
user={null}and nocategoryTypeto pin down the intended (fail-closed) behavior once the production logic is tightened per the companion comment inAlgorithmDropdown.jsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AlgorithmDropdown.test.jsx` around lines 112 - 133, Add a test in AlgorithmDropdown.test.jsx that covers the anonymous-user locked click path when categoryType is omitted, using AlgorithmDropdown and onLockedAlgorithmClick to verify the intended fail-closed behavior. Render with user={null} and no categoryType, click a locked option such as Quick Sort, and assert the callback is either not called or only called according to the tightened production logic so the default behavior is pinned down.src/i18n/locales/en/translation.json (1)
289-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing plural forms for
visualizationsRemaining.i18next requires
_one/_othersuffixes to correctly pluralize whencountis passed; without them, the same string is used for all values, producing "1 visualizations remaining" at the critical last-visualization moment for anonymous users. The same gap applies to thefrandarfiles.🌐 Proposed fix (apply analogous pattern to fr/ar)
- "visualizationsRemaining": "{{count}} visualizations remaining" + "visualizationsRemaining_one": "{{count}} visualization remaining", + "visualizationsRemaining_other": "{{count}} visualizations remaining"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/en/translation.json` around lines 289 - 290, The `visualizationsRemaining` translation is missing i18next plural variants, so add the `_one` and `_other` forms in the `translation.json` entry and apply the same pattern in the `fr` and `ar` locale files. Update the locale keys near `visualizationsRemaining` to use plural-aware entries so `count` resolves correctly for singular and plural cases, and keep the existing `noPath` key unchanged.src/components/AlgorithmDropdown.jsx (1)
121-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocked state isn't announced to assistive tech.
The
Lockicon isaria-hidden, so screen-reader users get no indication an option is locked until after activating it. Consider adding a visually-hidden label (e.g., "(locked)") to the button whenisLockedis true.♿ Proposed fix
{algo.label} </span> + {isLocked && ( + <span className="sr-only"> + {t('common.locked', 'locked')} + </span> + )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/AlgorithmDropdown.jsx` around lines 121 - 145, The locked state in AlgorithmDropdown is only shown via the Lock icon, which is hidden from assistive tech, so add an accessible text cue when isLocked is true. Update the item rendering path in the AlgorithmDropdown component so the same branch that renders the Lock icon also includes a visually hidden label like “(locked)” on the option/button, while keeping the icon itself decorative. Use the existing isLocked, Lock, and option rendering logic to locate the change.src/components/SettingsPanel.jsx (1)
367-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for size (array/grid) gating.
The slider and button gating logic here mirrors the speed-control pattern (intercept click/change, call
onGatedFeatureClick('category_controls'), block the underlyingonSizeChange), butSettingsPanel.test.jsxonly adds tests for manual mode, speed, and graph scenario gating — not for this size-control path. Since this is a new gated behavior on a critical settings surface, a regression here (e.g., accidentally callingonSizeChangebefore the gate check) would go undetected.Want me to draft tests asserting
onGatedFeatureClick('category_controls')fires (andonSizeChange/onArraySizeChangedoes not) when an anonymous user interacts with the size slider and size buttons?Also applies to: 418-444
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/SettingsPanel.jsx` around lines 367 - 403, Add test coverage in SettingsPanel.test.jsx for the size-control gating path in SettingsPanel’s size slider and size buttons. Verify that when canUseCategorySettings is false, interacting with the size control triggers onGatedFeatureClick('category_controls') and does not call the underlying onSizeChange or onArraySizeChange handlers. Mirror the existing speed-control gating tests so the regression surface around the size control stays covered.src/components/GraphScenarioDropdown.jsx (1)
106-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: locked options have no accessible indication besides a hidden icon.
Lockisaria-hidden, and there's noaria-disabled/description conveying the locked state to assistive tech; users only discover it after activating the option and triggering the sign-in prompt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/GraphScenarioDropdown.jsx` around lines 106 - 130, The locked option state in GraphScenarioDropdown is only shown with an aria-hidden Lock icon, so assistive tech gets no indication that the item is unavailable. Update the option rendering logic around isLocked and the Lock/Check display to expose the locked state via accessible semantics, such as aria-disabled on the option and a descriptive label or status text for locked items. Keep the current visual icon, but make sure the locked state is announced without relying on the hidden icon alone.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 102-107: The Free-tier entitlement text in the access list is
inconsistent with the PR objective: it currently says video export is unlimited
for Free users, but the shipped behavior should cap Free exports at 50 per day
with a mandatory watermark. Update the Free tier description in AGENTS.md to
match the actual export entitlement, keeping the Pro-tier note about watermark
customization/removal intact.
- Around line 51-53: The sound guidance is too broad and conflicts with the
ThemeToggle SFX added in this PR; update the rule in AGENTS.md to scope the
no-click-sound constraint to visualization playback only, or explicitly carve
out the ThemeToggle exception. Keep the guidance aligned with the existing sound
architecture references like soundEvents.js and soundManager so UI-specific
sounds are not universally blocked.
In `@src/index.css`:
- Around line 8-9: Stylelint is currently treating the new Tailwind v4 at-rules
in the index.css import block as unknown, so update the Stylelint configuration
to allow Tailwind-specific at-rules used by this file. Adjust the config for
scss/at-rule-no-unknown (or equivalent) to ignore `@config` and `@custom-variant`,
or switch to a Tailwind-aware Stylelint setup, so the existing Tailwind
directives in src/index.css lint cleanly.
In `@src/test/setup.js`:
- Around line 132-138: The mocked STATE_COLORS palette in the test setup has
AUXILIARY set to a different value than production, so update the test fixture
to match the production constant used by src/constants/index.js. Keep the change
in the state color mock aligned with the existing STATE_COLORS object so
snapshots and style assertions reflect runtime behavior.
In `@src/utils/themeSwitchSound.js`:
- Around line 63-92: The theme switch sound loader can get stuck on a rejected
promise, which prevents any later retry. Update ensureSwitchBuffers to clear
loadPromise whenever the async fetch/decode path fails, so playThemeSwitchSound
can attempt loading again on a future toggle; keep the existing caching for
success and make sure the failure reset happens around the loadPromise
assignment and the fetch/decodeAudioData flow.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 13-43: Keep AGENTS.md contract-only by removing the volatile
snapshot data from this section: the tooling/version counts, route table,
source/test counts, and source-of-truth registry listings should not live here.
Move those reference details to docs/AGENTS_REFERENCE.md, and keep AGENTS.md
limited to stable rules and workflow guidance so it doesn’t churn when
registries or counts change.
---
Nitpick comments:
In `@src/components/AlgorithmDropdown.jsx`:
- Around line 121-145: The locked state in AlgorithmDropdown is only shown via
the Lock icon, which is hidden from assistive tech, so add an accessible text
cue when isLocked is true. Update the item rendering path in the
AlgorithmDropdown component so the same branch that renders the Lock icon also
includes a visually hidden label like “(locked)” on the option/button, while
keeping the icon itself decorative. Use the existing isLocked, Lock, and option
rendering logic to locate the change.
In `@src/components/AlgorithmDropdown.test.jsx`:
- Around line 112-133: Add a test in AlgorithmDropdown.test.jsx that covers the
anonymous-user locked click path when categoryType is omitted, using
AlgorithmDropdown and onLockedAlgorithmClick to verify the intended fail-closed
behavior. Render with user={null} and no categoryType, click a locked option
such as Quick Sort, and assert the callback is either not called or only called
according to the tightened production logic so the default behavior is pinned
down.
In `@src/components/ArrayVisualizer.jsx`:
- Around line 106-123: Extract the duplicated complexity-gate behavior from
ArrayVisualizer’s effect block into a shared useComplexityGate(user, isComplete,
onGatedFeatureClick) hook, keeping the reset-on-completion, clear-on-user, and
count-once-per-completion logic in one place. Have the hook own the
hasCountedThisCompletion ref and isComplexityGated state, and update
GraphAlgorithmMatrixVisualizer, GridVisualizer, and TreeVisualizer to consume it
instead of inlining the same effects. Also pull the repeated overlay markup into
a small ComplexityGateOverlay component so all visualizers render the gate UI
consistently and stop drifting.
In `@src/components/GraphScenarioDropdown.jsx`:
- Around line 106-130: The locked option state in GraphScenarioDropdown is only
shown with an aria-hidden Lock icon, so assistive tech gets no indication that
the item is unavailable. Update the option rendering logic around isLocked and
the Lock/Check display to expose the locked state via accessible semantics, such
as aria-disabled on the option and a descriptive label or status text for locked
items. Keep the current visual icon, but make sure the locked state is announced
without relying on the hidden icon alone.
In `@src/components/SettingsPanel.jsx`:
- Around line 367-403: Add test coverage in SettingsPanel.test.jsx for the
size-control gating path in SettingsPanel’s size slider and size buttons. Verify
that when canUseCategorySettings is false, interacting with the size control
triggers onGatedFeatureClick('category_controls') and does not call the
underlying onSizeChange or onArraySizeChange handlers. Mirror the existing
speed-control gating tests so the regression surface around the size control
stays covered.
In `@src/i18n/locales/en/translation.json`:
- Around line 289-290: The `visualizationsRemaining` translation is missing
i18next plural variants, so add the `_one` and `_other` forms in the
`translation.json` entry and apply the same pattern in the `fr` and `ar` locale
files. Update the locale keys near `visualizationsRemaining` to use plural-aware
entries so `count` resolves correctly for singular and plural cases, and keep
the existing `noPath` key unchanged.
In `@src/services/entitlementService.js`:
- Around line 215-237: The free-tier export cap is only enforced in client-side
helpers, so it can be bypassed by clearing storage or changing browsers; update
the entitlement flow around canRunVideoExport, readDailyExportCount,
writeDailyExportCount, and incrementVideoExportCount so the 50/day limit is
backed by server-side accounting or validation rather than localStorage alone.
Apply the same fix approach to the anonymous counter logic in the related
visualization/complexity-view helpers so the product limit is enforced
consistently across sessions and devices.
- Around line 49-76: `readDailyExportCount` already defensively handles storage
failures, but `writeDailyExportCount` and the other entitlement helpers still
access `localStorage` directly and can throw in restricted environments. Add a
small shared safe-storage wrapper around `localStorage.getItem`/`setItem` and
use it from `writeDailyExportCount`, `incrementVisualizationCount`,
`incrementComplexityViewCount`, `canRunVisualization`,
`getRemainingVisualizations`, and `canViewComplexityPanel` so the entitlement
flow keeps working when storage is unavailable or quota-limited.
🪄 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 Plus
Run ID: 64f2869c-c34a-47d7-be61-defefada44cf
⛔ Files ignored due to path filters (1)
public/ui/sfx/switch-on.mp3is excluded by!**/*.mp3
📒 Files selected for processing (41)
AGENTS.mddocs/AGENTS_REFERENCE.mdsrc/components/AlgorithmDropdown.jsxsrc/components/AlgorithmDropdown.test.jsxsrc/components/ArrayVisualizer.complexityGate.test.jsxsrc/components/ArrayVisualizer.jsxsrc/components/ControlPanel.jsxsrc/components/ControlPanel.test.jsxsrc/components/GitHubRepoBadge.jsxsrc/components/GraphAlgorithmMatrixVisualizer.jsxsrc/components/GraphAlgorithmMatrixVisualizer.test.jsxsrc/components/GraphScenarioDropdown.jsxsrc/components/GraphScenarioDropdown.test.jsxsrc/components/GraphVisualizer.jsxsrc/components/GraphVisualizer.test.jsxsrc/components/GridVisualizer.jsxsrc/components/SettingsPanel.jsxsrc/components/SettingsPanel.test.jsxsrc/components/SignInPromptModal.jsxsrc/components/SignInPromptModal.test.jsxsrc/components/ThemeToggle.jsxsrc/components/ThemeToggle.test.jsxsrc/components/TreeVisualizer.jsxsrc/constants/__tests__/algorithmEntitlements.test.jssrc/constants/algorithmEntitlements.jssrc/contexts/AuthProvider.jsxsrc/contexts/AuthProvider.test.jsxsrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/index.csssrc/pages/VisualizerApp.jsxsrc/pages/VisualizerApp.test.jsxsrc/services/__tests__/entitlementService.test.jssrc/services/entitlementService.jssrc/services/entitlementService.test.jssrc/test/setup.jssrc/test/testUtils.jsxsrc/utils/themeSwitchSound.jssrc/utils/themeSwitchSound.test.jssrc/video/useVideoExporter.js
💤 Files with no reviewable changes (1)
- src/services/entitlementService.test.js
| - **Sound:** semantic events in `soundEvents.js` only (16 event kinds) — not from localized descriptions; visualization-only (no UI click sounds); uses Tone.js singleton `soundManager` with 5 synths through master chain (gain → filter → compressor → reverb) | ||
| - **Export:** interactive + Remotion parity; `buildExportSoundCues()` from same sound events; WAV assets in `public/video-export/sfx/` (18 pre-rendered files) | ||
| - **Feature gating:** Tiered access model — see Auth contracts for Anonymous vs Free tier access; `SignInPromptModal` blocks gated features; `entitlementService.js` is the single authority for all access checks; `src/constants/algorithmEntitlements.js` defines the anonymous-tier algorithm allowlist |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Scope the no-click-sound rule.
This blanket ban now conflicts with the ThemeToggle SFX added elsewhere in the PR. Either carve out that exception or narrow the rule to visualization playback only.
Based on learnings, sound must remain visualization-only and not UI click sounds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 51 - 53, The sound guidance is too broad and
conflicts with the ThemeToggle SFX added in this PR; update the rule in
AGENTS.md to scope the no-click-sound constraint to visualization playback only,
or explicitly carve out the ThemeToggle exception. Keep the guidance aligned
with the existing sound architecture references like soundEvents.js and
soundManager so UI-specific sounds are not universally blocked.
Source: Learnings
| - **Free tier (Google sign-in):** | ||
| - All 45 algorithms, unlimited visualizations | ||
| - Manual controls, all 4 speed presets | ||
| - Full complexity panel access, all category-specific controls | ||
| - Code Panel, Insight Panel, Sound, Fullscreen | ||
| - Video Export: unlimited for Free tier with mandatory watermark; Pro tier adds watermark customization/removal; internal daily abuse guard exists (not user-facing) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the Free-tier export cap.
This says Free export is unlimited, but the PR objective limits Free users to 50 exports/day with a mandatory watermark. The entitlement contract here needs to match the shipped behavior.
Based on the PR objective, Free users are limited to 50 exports/day.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 102 - 107, The Free-tier entitlement text in the
access list is inconsistent with the PR objective: it currently says video
export is unlimited for Free users, but the shipped behavior should cap Free
exports at 50 per day with a mandatory watermark. Update the Free tier
description in AGENTS.md to match the actual export entitlement, keeping the
Pro-tier note about watermark customization/removal intact.
| @config "../tailwind.config.js"; | ||
| @custom-variant dark (&:where(.dark, .dark *)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Stylelint will fail on new Tailwind v4 at-rules.
@config and @custom-variant are Tailwind v4 at-rules not recognized by the current Stylelint config (scss/at-rule-no-unknown), per the static analysis hints. This will break the lint pipeline unless the config is updated to allow these at-rules (e.g. via ignoreAtRules or a Tailwind-aware Stylelint syntax/plugin).
🔧 Suggested Stylelint config update
rules: {
+ 'at-rule-no-unknown': [true, { ignoreAtRules: ['config', 'custom-variant', 'utility', 'theme', 'plugin', 'apply'] }],
+ 'scss/at-rule-no-unknown': null,🧰 Tools
🪛 Stylelint (17.14.0)
[error] 8-8: Unexpected unknown at-rule "@config" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 9-9: Unexpected unknown at-rule "@custom-variant" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/index.css` around lines 8 - 9, Stylelint is currently treating the new
Tailwind v4 at-rules in the index.css import block as unknown, so update the
Stylelint configuration to allow Tailwind-specific at-rules used by this file.
Adjust the config for scss/at-rule-no-unknown (or equivalent) to ignore `@config`
and `@custom-variant`, or switch to a Tailwind-aware Stylelint setup, so the
existing Tailwind directives in src/index.css lint cleanly.
Source: Linters/SAST tools
| STATE_COLORS: { | ||
| default: '#e5e7eb', | ||
| comparing: '#fbbf24', | ||
| swapping: '#f97316', | ||
| sorted: '#10b981', | ||
| pivot: '#8b5cf6', | ||
| auxiliary: '#9ca3af', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the AUXILIARY mock color aligned with production.
src/constants/index.js already defines AUXILIARY as #6b7280, but this test setup uses #9ca3af. If any snapshot or style assertions rely on the mocked palette, they’ll drift from runtime behavior.
♻️ Suggested fix
- auxiliary: '`#9ca3af`',
+ auxiliary: '`#6b7280`',📝 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.
| STATE_COLORS: { | |
| default: '#e5e7eb', | |
| comparing: '#fbbf24', | |
| swapping: '#f97316', | |
| sorted: '#10b981', | |
| pivot: '#8b5cf6', | |
| auxiliary: '#9ca3af', | |
| STATE_COLORS: { | |
| default: '`#e5e7eb`', | |
| comparing: '`#fbbf24`', | |
| swapping: '`#f97316`', | |
| sorted: '`#10b981`', | |
| pivot: '`#8b5cf6`', | |
| auxiliary: '`#6b7280`', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/setup.js` around lines 132 - 138, The mocked STATE_COLORS palette in
the test setup has AUXILIARY set to a different value than production, so update
the test fixture to match the production constant used by
src/constants/index.js. Keep the change in the state color mock aligned with the
existing STATE_COLORS object so snapshots and style assertions reflect runtime
behavior.
| async function ensureSwitchBuffers() { | ||
| if (switchOnBuffer && switchOffBuffer) { | ||
| return { on: switchOnBuffer, off: switchOffBuffer }; | ||
| } | ||
|
|
||
| if (!loadPromise) { | ||
| loadPromise = (async () => { | ||
| const context = getAudioContext(); | ||
| if (!context) { | ||
| return null; | ||
| } | ||
|
|
||
| const response = await fetch(SWITCH_SOUND_URL); | ||
| if (!response.ok) { | ||
| throw new Error( | ||
| `Failed to load theme switch sound (${response.status})` | ||
| ); | ||
| } | ||
|
|
||
| const arrayBuffer = await response.arrayBuffer(); | ||
| const decoded = await context.decodeAudioData(arrayBuffer); | ||
| switchOnBuffer = decoded; | ||
| switchOffBuffer = reverseAudioBuffer(decoded, context); | ||
|
|
||
| return { on: switchOnBuffer, off: switchOffBuffer }; | ||
| })(); | ||
| } | ||
|
|
||
| return loadPromise; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Failed load permanently disables the sound.
If fetch/decodeAudioData rejects (e.g. transient network error, non-ok response), loadPromise stays set to the rejected promise. Every later playThemeSwitchSound call reuses that same rejected promise via the if (!loadPromise) guard, so the sound is silently broken for the rest of the session — it never retries.
🔧 Proposed fix: reset loadPromise on failure so a later toggle retries
if (!loadPromise) {
loadPromise = (async () => {
const context = getAudioContext();
if (!context) {
return null;
}
- const response = await fetch(SWITCH_SOUND_URL);
- if (!response.ok) {
- throw new Error(
- `Failed to load theme switch sound (${response.status})`
- );
- }
-
- const arrayBuffer = await response.arrayBuffer();
- const decoded = await context.decodeAudioData(arrayBuffer);
- switchOnBuffer = decoded;
- switchOffBuffer = reverseAudioBuffer(decoded, context);
-
- return { on: switchOnBuffer, off: switchOffBuffer };
+ try {
+ const response = await fetch(SWITCH_SOUND_URL);
+ if (!response.ok) {
+ throw new Error(
+ `Failed to load theme switch sound (${response.status})`
+ );
+ }
+
+ const arrayBuffer = await response.arrayBuffer();
+ const decoded = await context.decodeAudioData(arrayBuffer);
+ switchOnBuffer = decoded;
+ switchOffBuffer = reverseAudioBuffer(decoded, context);
+
+ return { on: switchOnBuffer, off: switchOffBuffer };
+ } catch (error) {
+ loadPromise = null;
+ throw error;
+ }
})();
}📝 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.
| async function ensureSwitchBuffers() { | |
| if (switchOnBuffer && switchOffBuffer) { | |
| return { on: switchOnBuffer, off: switchOffBuffer }; | |
| } | |
| if (!loadPromise) { | |
| loadPromise = (async () => { | |
| const context = getAudioContext(); | |
| if (!context) { | |
| return null; | |
| } | |
| const response = await fetch(SWITCH_SOUND_URL); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Failed to load theme switch sound (${response.status})` | |
| ); | |
| } | |
| const arrayBuffer = await response.arrayBuffer(); | |
| const decoded = await context.decodeAudioData(arrayBuffer); | |
| switchOnBuffer = decoded; | |
| switchOffBuffer = reverseAudioBuffer(decoded, context); | |
| return { on: switchOnBuffer, off: switchOffBuffer }; | |
| })(); | |
| } | |
| return loadPromise; | |
| } | |
| async function ensureSwitchBuffers() { | |
| if (switchOnBuffer && switchOffBuffer) { | |
| return { on: switchOnBuffer, off: switchOffBuffer }; | |
| } | |
| if (!loadPromise) { | |
| loadPromise = (async () => { | |
| const context = getAudioContext(); | |
| if (!context) { | |
| return null; | |
| } | |
| try { | |
| const response = await fetch(SWITCH_SOUND_URL); | |
| if (!response.ok) { | |
| throw new Error( | |
| `Failed to load theme switch sound (${response.status})` | |
| ); | |
| } | |
| const arrayBuffer = await response.arrayBuffer(); | |
| const decoded = await context.decodeAudioData(arrayBuffer); | |
| switchOnBuffer = decoded; | |
| switchOffBuffer = reverseAudioBuffer(decoded, context); | |
| return { on: switchOnBuffer, off: switchOffBuffer }; | |
| } catch (error) { | |
| loadPromise = null; | |
| throw error; | |
| } | |
| })(); | |
| } | |
| return loadPromise; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/themeSwitchSound.js` around lines 63 - 92, The theme switch sound
loader can get stuck on a rejected promise, which prevents any later retry.
Update ensureSwitchBuffers to clear loadPromise whenever the async fetch/decode
path fails, so playThemeSwitchSound can attempt loading again on a future
toggle; keep the existing caching for success and make sure the failure reset
happens around the loadPromise assignment and the fetch/decodeAudioData flow.
✅ Action performedReview finished.
|
…eportExportError, and remove dead null branch in GitHubRepoBadge
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/GraphVisualizer.test.jsx (1)
183-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame missing localStorage cleanup as the sibling test in
GraphAlgorithmMatrixVisualizer.test.jsx.
anon_complexity_viewsis set vialocalStorage.setItembut never removed. As established, Vitest only resets global state (localStorage included) between files, not between tests in the same file, so this leaks into later tests in this suite if any are added or reordered.🧹 Proposed fix
} finally { vi.useRealTimers(); + localStorage.removeItem('anon_complexity_views'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/GraphVisualizer.test.jsx` around lines 183 - 205, The anonymous complexity view test is leaking localStorage state by setting anon_complexity_views without cleaning it up afterward. Update the GraphVisualizer.test.jsx test case that uses renderGraph and vi.useFakeTimers to remove the anon_complexity_views key in the test teardown, matching the cleanup pattern used elsewhere in the suite so later tests in the same file are not affected.src/components/GraphAlgorithmMatrixVisualizer.test.jsx (1)
98-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing localStorage cleanup after the test.
localStorage.setItem('anon_complexity_views', '2')is never cleared. Since Vitest shares global state (includinglocalStorage) across tests within the same file and only resets between files, this value persists into subsequent tests in this suite. The next test happens to not exerciseisComplete, so it's currently harmless, but this is fragile against future test additions or reordering.🧹 Proposed fix
try { localStorage.setItem('anon_complexity_views', '2'); renderWithProviders( ... ); act(() => { vi.advanceTimersByTime(1000); }); const blurOverlay = document.querySelector('.backdrop-blur-md'); expect(blurOverlay).toBeInTheDocument(); expect(screen.getByText('Complexity Analysis')).toBeInTheDocument(); } finally { vi.useRealTimers(); + localStorage.removeItem('anon_complexity_views'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/GraphAlgorithmMatrixVisualizer.test.jsx` around lines 98 - 129, The test in GraphAlgorithmMatrixVisualizer.test.jsx leaves the anon_complexity_views localStorage value behind, which can leak state into later tests. Update the anonymous user overlay test to clean up that key after rendering and assertions, ideally in the existing finally block alongside vi.useRealTimers, so the GraphAlgorithmMatrixVisualizer suite stays isolated and repeatable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/GraphAlgorithmMatrixVisualizer.test.jsx`:
- Around line 98-129: The test in GraphAlgorithmMatrixVisualizer.test.jsx leaves
the anon_complexity_views localStorage value behind, which can leak state into
later tests. Update the anonymous user overlay test to clean up that key after
rendering and assertions, ideally in the existing finally block alongside
vi.useRealTimers, so the GraphAlgorithmMatrixVisualizer suite stays isolated and
repeatable.
In `@src/components/GraphVisualizer.test.jsx`:
- Around line 183-205: The anonymous complexity view test is leaking
localStorage state by setting anon_complexity_views without cleaning it up
afterward. Update the GraphVisualizer.test.jsx test case that uses renderGraph
and vi.useFakeTimers to remove the anon_complexity_views key in the test
teardown, matching the cleanup pattern used elsewhere in the suite so later
tests in the same file are not affected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 302462e9-26e1-4cf4-b60f-f4198e5c3abc
📒 Files selected for processing (5)
src/components/GitHubRepoBadge.jsxsrc/components/GraphAlgorithmMatrixVisualizer.test.jsxsrc/components/GraphVisualizer.test.jsxsrc/pages/VisualizerApp.test.jsxsrc/video/useVideoExporter.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/VisualizerApp.test.jsx
- src/components/GitHubRepoBadge.jsx
1 participant
Contribution workflow
develop: This PR targetsdevelop, notmain.Description
Implements a tiered access model across the platform, introducing Anonymous and Free user tiers with differentiated capabilities. Establishes the entitlement service as the single authority for all access checks, gates visualizer controls, limits anonymous usage, enforces video export restrictions, and adds a theme switch sound effect.
Type of Change
Related Issues
N/A
Changes Made
entitlementService.jsfrom async Supabase-based plan fetch to synchronous tiered access model withgetUserPlan(),canAccessAlgorithm(),canUseManualControls(),canChangeSpeed(),canUseCategoryControls(),canRunVisualization(),canViewComplexityPanel(),canRunVideoExport(),getExportWatermarkConfig(),canCustomizeExportWatermark(), andresetAllSessionCounters(). Anonymous session limits (12 visualizations, 2 complexity views) tracked via localStorage.algorithmEntitlements.jsdefining the anonymous-tier allowlist — 18 of 45 algorithms across all 5 categories.AlgorithmDropdownfor restricted algorithms; gated manual mode, speed slider, and category controls inSettingsPanel; gated sort-order toggle inControlPanel; locked graph scenarios inGraphScenarioDropdown— all for anonymous users.ArrayVisualizer,GraphVisualizer,GridVisualizer,TreeVisualizer,GraphAlgorithmMatrixVisualizer).SignInPromptModalto support context-aware feature gates with dynamic titles, descriptions, and metadata interpolation (e.g., algorithm name, session limit).themeSwitchSound.jswith Web Audio-based light-switch click — forward playback for light mode, reversed + slowed (0.82x) for dark mode. Respects prefers-reduced-motion.GitHubRepoBadgein Tooltip; updatedindex.csswith Tailwind 4 directives (@utility text-theme-primary); added 10 new i18n keys across en/fr/ar; comprehensive updates toAGENTS.md/AGENTS_REFERENCE.md.AuthProvidercallsresetAllSessionCounters()on SIGNED_IN event;VisualizerAppintegrates entitlement checks into play/sound/fullscreen/export handlers and forces autoplay mode + MEDIUM grid for anonymous users.Testing
Test Results
3 new test files added (entitlementService, complexity gate, themeSwitchSound) + 10+ updated test suites across all gated components.
Code Quality
pnpm lint)pnpm format)Performance Impact
Accessibility
Breaking Changes
None. Anonymous users experience reduced functionality (18 algorithms, 12 visualizations, autoplay only). Existing signed-in users remain unaffected.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation