Supporter plan, cloud backend, and SEO pages (1.1.0) - #1
Merged
Conversation
Adds an optional account and Supporter tier on top of the existing editor, plus build-time SEO landing pages. The core editor stays free and fully offline: with none of the new environment variables set, every cloud feature hides itself and the app behaves exactly as before. Accounts and cloud (Supabase) - Email + password and Google sign-in, with password reset. - Cloud sync for graphs, projects and custom templates, with version history. - View-only share links on unguessable slugs, resolved anonymously through a get_share() RPC so the shares table is never readable by anon. - Full schema in supabase/schema.sql with RLS on every table. Entitlement is one rule, pro_until > now(), enforced identically in SQL and TypeScript. Hosted AI - /api/generate runs generation server-side for supporters. - Three interchangeable backends, first configured wins: Vertex AI express key, Vertex AI with a project (ADC locally, service account on Vercel), or a Google AI Studio key. - Metered per user per month via atomic SQL, default 150. Upstream failures are refunded; a response that arrives but fails to parse is not, so it cannot be farmed. - Free users keep unlimited generation with their own key. Billing (Polar) - Checkout, customer portal, and a signature-verified webhook that is the only writer of billing columns. - Renewal is cushioned by a 1-day margin and never moved backward by a delayed or out-of-order event; cancellation still ends access immediately. - Account deletion cancels any live subscription before deleting. SEO - 12 static diagram pages plus a hub and sitemap.xml, generated at build time into dist/. public/sitemap.xml is removed because it is now generated. Ops - db-keepalive.yml pings the database every ~5 days so a free-tier Supabase project never pauses; update-supporters.yml refreshes the supporters list. Licensing - AGPL-3.0, with the section 13 source offer linked from Settings. The project name, logo and branding are reserved separately from the code licence, so forks run under their own branding. - Privacy Policy and Terms pages, governed by Finnish law and preserving EU/EEA consumer rights.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Sukarth, your pull request is larger than the review limit of 150000 diff characters
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe release adds optional Supporter features backed by Supabase, Polar, and hosted AI, including authentication, cloud synchronization, version history, sharing, templates, billing, account deletion, and new public pages. It also adds SEO page generation, deployment configuration, documentation, and scheduled maintenance workflows. Supporter cloud platform
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Reviewer's GuideAdds optional cloud-backed Supporter features (accounts, sync, hosted AI, billing, sharing, custom templates) plus static SEO pages, while preserving the existing fully local free editor behavior when backend env vars are absent. Sequence diagram for hosted AI generation via /api/generatesequenceDiagram
participant App
participant HostedClient as hostedAi.generateDiagramDataHosted
participant Api as /api/generate
participant Supa as Supabase
participant Gemini as GoogleGenAI
App->>HostedClient: generateDiagramDataHosted(prompt, history)
HostedClient->>Api: POST /api/generate
Api->>Supa: getUserFromRequest(token)
Api->>Supa: getProfile(user.id)
Api->>Supa: increment_ai_usage(p_user, p_month, p_limit)
alt quota exceeded
Supa-->>Api: newCount = -1
Api-->>HostedClient: 429 quota_exceeded
HostedClient-->>App: Error (quota exceeded)
else within quota
Api->>Gemini: models.generateContent(model, contents, config)
alt upstream failure
Gemini-->>Api: [error]
Api->>Supa: refund_ai_usage(p_user, p_month)
Api-->>HostedClient: 502 error
HostedClient-->>App: Error (hosted AI failed)
else success
Gemini-->>Api: response.text (diagram JSON)
Api-->>HostedClient: 200 { diagram, usage }
HostedClient-->>App: DiagramData
end
end
Sequence diagram for Polar subscription webhook updating entitlementsequenceDiagram
participant Polar as Polar
participant Webhook as /api/webhooks/polar
participant SupaAdmin as SupabaseAdmin
Polar->>Webhook: POST subscription.* (signed)
Webhook->>Webhook: validateEvent(rawBody, headers, POLAR_WEBHOOK_SECRET)
alt subscription event
Webhook->>SupaAdmin: getProfile(userId)
alt status in {active,trialing,past_due}
Webhook->>SupaAdmin: update profiles
Note right of SupaAdmin: pro_status = status
Note right of SupaAdmin: pro_until = max(current, period_end + margin)
else terminal status (canceled,revoked,unpaid)
Webhook->>SupaAdmin: update profiles
Note right of SupaAdmin: pro_status = status
Note right of SupaAdmin: pro_until = now()
end
else other event
Webhook-->>Polar: 202 received
end
Webhook-->>Polar: 202 received
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary by QodoAdd Supporter plan: Supabase accounts/sync, hosted AI, Polar billing, and SEO pages
AI Description
Diagram
High-Level Assessment
Files changed (54)
|
There was a problem hiding this comment.
Pull request overview
This PR introduces an optional Supporter tier with cloud-backed features (Supabase auth/sync/sharing, Polar billing, and Vercel serverless endpoints) while preserving the existing “offline-first, free editor” behavior when no backend env vars are configured. It also adds new SEO/marketing/legal pages plus ops automation (keepalive + supporters refresh) and bumps the app to 1.1.0.
Changes:
- Add Supabase-backed accounts, entitlement gating, cloud sync + version history, share links, and custom templates.
- Add hosted AI and billing flows via Vercel serverless functions (Polar checkout/portal/webhook + usage metering).
- Add pricing/compare/legal pages and supporting docs/workflows; update build and deployment routing.
Reviewed changes
Copilot reviewed 52 out of 55 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Adds a dev-only Vite middleware to run api/* handlers locally; updates server host settings. |
| vite-env.d.ts | Declares Vite env typings for Supabase client configuration. |
| vercel.json | Updates routing/rewrites behavior for SPA vs /api/*. |
| supabase/schema.sql | Introduces full Supabase schema with RLS, entitlement helper functions, share RPC, and usage metering functions. |
| services/useCloudSync.ts | Adds a debounced, self-healing cloud sync React hook. |
| services/sync.ts | Implements local-first sync engine, tombstones, version snapshotting, and share refresh logic. |
| services/supabaseClient.ts | Adds optional Supabase client initialization (null when unconfigured). |
| services/shares.ts | Implements share slug generation, share upserts, revoke, and public payload fetch via RPC. |
| services/openrouter.ts | Refactors key storage to shared obfuscation helper. |
| services/keyObfuscation.ts | Adds shared key obfuscation helpers for BYOK providers. |
| services/hostedAi.ts | Adds client wrapper for hosted AI generation + usage fetch. |
| services/gemini.ts | Refactors prompt/schema to shared module and key obfuscation helper. |
| services/entitlement.ts | Adds shared entitlement predicate (pro_until > now). |
| services/diagramPrompt.ts | Centralizes Gemini system prompt and JSON schema for client+server reuse. |
| services/customTemplates.ts | Adds cloud-synced custom templates and graph version history fetch. |
| services/cloudErrors.ts | Adds shared RLS-denial detection for nicer UX messaging. |
| services/billing.ts | Adds client billing helpers for checkout/portal/account deletion. |
| services/auth.tsx | Adds Supabase auth/profile context, recovery mode, and profile update helpers. |
| services/aiProvider.ts | Adds “hosted” as an AI provider option and display name logic. |
| services/ai.ts | Routes diagram generation to hosted AI when selected; updates “has key” logic. |
| scripts/update-supporters.mjs | Adds script to refresh README supporters list from Supabase. |
| README.md | Updates docs for Supporter plan, free-forever guarantee, architecture, and new pages. |
| public/sitemap.xml | Removes static sitemap (now generated at build time). |
| package.json | Bumps version to 1.1.0; adds build step for SEO generation and new dependencies. |
| index.tsx | Wraps app in AuthProvider. |
| index.html | Updates SEO titles for OG/Twitter meta tags. |
| docs/BACKEND_SETUP.md | Adds comprehensive self-hosting guide for Supabase/Polar/hosted AI. |
| components/ShareModal.tsx | Adds UI flow for creating/copying/revoking share links (Supporter-gated). |
| components/SharedViewPage.tsx | Adds public read-only shared view route UI for graphs/projects. |
| components/SettingsPage.tsx | Adds Account & Cloud section integration and hosted AI provider UX. |
| components/PricingPage.tsx | Adds pricing page with free-vs-supporter messaging and checkout flow. |
| components/LegalPages.tsx | Adds Privacy Policy + Terms pages. |
| components/LandingPage.tsx | Updates landing page nav/CTAs and licensing/support messaging. |
| components/ComponentLibrary.tsx | Adds Supporter custom template library UI (save/delete/sync). |
| components/ComparePage.tsx | Adds a comparison page vs other tools. |
| components/CloudHistoryModal.tsx | Adds UI to browse and restore cloud version history (Supporter-gated). |
| components/AuthModal.tsx | Adds auth modal (signin/signup/forgot + Google OAuth). |
| CHANGELOG.md | Adds 1.1.0 release notes. |
| api/webhooks/polar.ts | Adds signature-verified Polar webhook to maintain entitlement state. |
| api/usage.ts | Adds hosted AI usage endpoint. |
| api/portal.ts | Adds billing portal endpoint. |
| api/generate.ts | Adds hosted AI generation endpoint with entitlement + monthly metering. |
| api/delete-account.ts | Adds account deletion endpoint that cancels billing then deletes user. |
| api/checkout.ts | Adds checkout endpoint with safeguards against double-subscription. |
| api/_lib/supabaseAdmin.ts | Adds server-side Supabase admin helpers and entitlement/month helpers. |
| api/_lib/polar.ts | Adds Polar client helper and robust app URL resolution for redirects. |
| .gitignore | Ignores .vercel and .env* (except .env.example). |
| .github/workflows/update-supporters.yml | Adds scheduled workflow to refresh README supporters list. |
| .github/workflows/db-keepalive.yml | Adds scheduled workflow to keep free-tier Supabase DB from pausing. |
| .env.example | Adds documented env var template for optional backend features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+124
to
+127
| define: { | ||
| 'process.env.API_KEY': JSON.stringify(env.GEMINI_API_KEY), | ||
| 'process.env.GEMINI_API_KEY': JSON.stringify(env.GEMINI_API_KEY) | ||
| }, |
Comment on lines
+6
to
7
| "source": "/((?!api/).*)", | ||
| "destination": "/index.html" |
Comment on lines
+39
to
+40
| const used = usageResult.data?.count ?? 0; | ||
| return res.status(200).json({ |
| "version": "1.1.0", | ||
| "description": "Free and open-source AI-powered economics diagram editor built for IB students and educators.", | ||
| "type": "module", | ||
| "license": "MIT", |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/SettingsPage.tsx (1)
394-404: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPersisted
hostedprovider leaves the page in a dead state when cloud isn't configured.If
hostedwas stored previously (or the deployment drops the Supabase env vars),providerstays'hosted'while its<option>isn't rendered: the select shows blank, both the key and model sections are hidden, and AI generation has no working provider. Consider falling back to a BYOK provider on mount when!cloudConfigured.🐛 Sketch of the fallback
useEffect(() => { const p = getAIProvider(); - setProviderState(p); - loadProviderState(p); - }, []); + const effective = p === 'hosted' && !cloudConfigured ? 'gemini' : p; + if (effective !== p) setAIProvider(effective); + setProviderState(effective); + loadProviderState(effective); + }, [cloudConfigured]);🤖 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 `@components/SettingsPage.tsx` around lines 394 - 404, Update the provider initialization in SettingsPage so a persisted "hosted" value is replaced with a supported BYOK provider when cloudConfigured is false. Ensure this fallback runs on mount or when configuration is loaded, while preserving the existing provider selection when hosted is available and keeping the select, key fields, and model sections synchronized.App.tsx (1)
690-721: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMove
fetchCloudIds()before the state updates.
setGraphs/setProjectsat Lines 702-703 immediately schedule a debounced cloud sync, while the cloud-only tombstones are only recorded after the awaited network round trip. If the sync fires first, exactly the resurrection this block guards against can still happen. Fetching the ids up front closes the window.🛠️ Suggested reordering
const importedGraphIds = new Set(data.graphs.map(g => g.id)); const importedProjectIds = new Set(data.projects.map(p => p.id)); + // Best-effort: null when offline. Done before mutating state so the + // debounced sync can't start with an incomplete tombstone set. + const cloud = await fetchCloudIds(); + if (cloud) { + recordTombstones('graphs', cloud.graphIds.filter(id => !importedGraphIds.has(id))); + recordTombstones('projects', cloud.projectIds.filter(id => !importedProjectIds.has(id))); + } recordTombstones('graphs', graphs.filter(g => !importedGraphIds.has(g.id)).map(g => g.id));and drop the trailing
fetchCloudIds()block at Lines 712-720.🤖 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 `@App.tsx` around lines 690 - 721, Update handleImportData so fetchCloudIds() runs before setGraphs and setProjects, then record tombstones for cloud-only graph and project IDs using the imported ID sets. Remove the trailing fetchCloudIds block after the state updates, preserving the existing best-effort null handling.
🟡 Minor comments (15)
scripts/seo-content.mjs-454-458 (1)
454-458: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale MIT license claim.
Line 457 says the project is MIT licensed, but the README and changelog now declare AGPL-3.0. This publishes contradictory licensing information on the PPC page.
🤖 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 `@scripts/seo-content.mjs` around lines 454 - 458, Update the licensing statement in the faq array of seo-content.mjs to reference AGPL-3.0 instead of MIT, keeping the rest of the classroom-use answer unchanged.docs/BACKEND_SETUP.md-163-165 (1)
163-165: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReconcile the billing grace-period documentation.
Line 164 specifies a 3-day grace period, while
CHANGELOG.mdLines 54-56 says 1 day. Document the implemented value consistently before operators configure billing expectations.🤖 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 `@docs/BACKEND_SETUP.md` around lines 163 - 165, Reconcile the grace-period value described near the webhook entitlement documentation with the implemented billing behavior and the corresponding CHANGELOG entry. Update the conflicting documentation so the stated grace period is consistent across both references, preserving the existing entitlement and renewal semantics.docs/BACKEND_SETUP.md-101-105 (1)
101-105: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify languages for the environment-variable code blocks.
These fences trigger the reported MD040 warnings. Mark them as
dotenvto keep documentation lint-clean.Proposed fix
-``` +```dotenv VERTEX_API_KEY=... ...</details> Also applies to: 121-127, 132-136 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/BACKEND_SETUP.mdaround lines 101 - 105, Update the
environment-variable code fences in BACKEND_SETUP.md, including the blocks
around VERTEX_API_KEY and the additional referenced blocks, to specify the
dotenv language. Preserve their existing contents and formatting while ensuring
every affected fence is marked for dotenv syntax.</details> <!-- cr-comment:v1:38c8396769f02cec77cca658 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>api/_lib/polar.ts-32-40 (1)</summary><blockquote> `32-40`: _🔒 Security & Privacy_ | _🟡 Minor_ | _⚡ Quick win_ **Do not use arbitrary request origins for payment redirects.** Line 32 accepts a caller-controlled `Origin`; without `APP_URL`, an authenticated checkout can be configured to return to an attacker-controlled domain. Require `APP_URL` outside explicit local development, or validate against a fixed allowlist. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@api/_lib/polar.tsaround lines 32 - 40, Update the origin-selection logic
around the origin header and fallback URL so payment redirects never use an
arbitrary caller-controlled Origin. Require the configured APP_URL (or an
equivalent fixed allowlist) for non-local environments, while preserving direct
localhost/loopback handling for explicit local development; ensure authenticated
checkout cannot fall back to an untrusted request origin.</details> <!-- cr-comment:v1:396ced5f66a22355f5bf7de7 --> </blockquote></details> <details> <summary>components/LegalPages.tsx-37-39 (1)</summary><blockquote> `37-39`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_ **Increase contrast for muted metadata and footer text.** `text-gray-400` on white is insufficient for normal-size text under WCAG AA’s 4.5:1 minimum. Use `text-gray-500` or darker for the update date and footer’s resting state. ([w3.org](https://www.w3.org/TR/WCAG22/?utm_source=openai)) <details> <summary>Proposed fix</summary> ```diff - <p className="text-sm text-gray-400 mb-10">Last updated: {LAST_UPDATED}</p> + <p className="text-sm text-gray-500 mb-10">Last updated: {LAST_UPDATED}</p> ... - <footer className="mt-16 pt-8 border-t border-slate-100 text-sm text-gray-400 flex flex-wrap gap-x-6 gap-y-2"> + <footer className="mt-16 pt-8 border-t border-slate-100 text-sm text-gray-500 flex flex-wrap gap-x-6 gap-y-2">🤖 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 `@components/LegalPages.tsx` around lines 37 - 39, Update the Last updated paragraph and footer in the LegalPages component to use text-gray-500 or a darker text color instead of text-gray-400, preserving the existing layout and other styling.Source: MCP tools
components/ComponentLibrary.tsx-247-271 (1)
247-271: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCustom template rows aren't keyboard reachable.
The row is a
divwithonClickonly, so keyboard users can't add a saved template (the nested deletebuttonis focusable, the row itself isn't). Addrole="button",tabIndex={0}, and an Enter/Space handler — or render the row content as abutton.🤖 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 `@components/ComponentLibrary.tsx` around lines 247 - 271, Update the custom template rows rendered in filteredCustom.map to be keyboard accessible: add button semantics, make each row focusable, and handle Enter and Space by invoking addCustomTemplate(t), while preserving the nested delete button’s stopPropagation behavior.components/AccountSection.tsx-82-106 (1)
82-106: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the checkout poll attempt counter stable across
refreshProfilerecreations.
refreshProfileis only memoized tofetchProfile; iffetchProfiledepends on values that can change during checkout polling, this effect can tear down and restart, resetting the localattemptscounter and preventing the “taking longer” fallback. Move the attempt count into a ref or otherwise keep the counter stable across effect iterations.🤖 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 `@components/AccountSection.tsx` around lines 82 - 106, The checkout poll attempt counter in the useEffect must persist when refreshProfile changes and the effect restarts. Move attempts to a useRef (or equivalent stable state), reset it when a new checkout begins, increment the stable value during polling, and use it for the delayed fallback while preserving the existing cleanup behavior.components/AuthModal.tsx-165-196 (1)
165-196: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEmail/password inputs lack accessible names.
Only placeholders are provided (no
<label>oraria-label), so screen-reader users get no reliable field name. Addaria-label(or visually-hidden labels) to both inputs, and to the reset-form email field at Line 127.🤖 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 `@components/AuthModal.tsx` around lines 165 - 196, Add accessible names to both email and password inputs in the AuthModal form, and to the reset-form email input near the reset flow. Use clear aria-labels or existing visually hidden labels, while preserving the current input behavior and autocomplete settings.components/ComponentLibrary.tsx-213-213 (1)
213-213: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnter key bypasses the disabled-state guard.
handleSaveTemplatehas no internalsaving/empty-name check, so pressing Enter repeatedly can fire concurrent inserts (creating duplicate templates) or submit a blank name. Guard inside the handler.🐛 Proposed fix
const handleSaveTemplate = async () => { + if (saving || !saveName.trim()) return; if (!user) {🤖 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 `@components/ComponentLibrary.tsx` at line 213, Update handleSaveTemplate in ComponentLibrary so it immediately returns when a save is already in progress or the template name is empty, ensuring both button clicks and the Enter-key onKeyDown path share the same guard and cannot create duplicate or blank templates.App.tsx-822-828 (1)
822-828: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTitle becomes permanently AI-immutable after the first generation.
userNamedis true for any title other thanEMPTY_DIAGRAM.title, and the first generation writes the AI title back intograph.title(Line 844). Every later prompt therefore keeps the first diagram's title even though the user never renamed anything. If the intent is "respect explicit renames", track that with a flag set byrenameGraphrather than inferring it from the current title.🤖 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 `@App.tsx` around lines 822 - 828, Replace the title-based userNamed inference in the generation flow with an explicit flag recorded by renameGraph. Set that flag only when the user renames the graph, and use it when deciding whether to preserve activeGraph.title, so AI-generated titles remain updateable until an explicit rename occurs.components/SharedViewPage.tsx-168-174 (1)
168-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStray space before the comma in the footer.
{' '}followed by a line beginning with,renders as "IB EconGraph AI , the free, open-source…" on a public, link-shared page.✏️ Fix
<button onClick={onGoHome} className="text-blue-600 hover:underline font-medium"> IB EconGraph AI - </button>{' '} - , the free, open-source economics diagram editor for IB students. + </button> + , the free, open-source economics diagram editor for IB students.🤖 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 `@components/SharedViewPage.tsx` around lines 168 - 174, Remove the whitespace expression following the “IB EconGraph AI” button in the footer of SharedViewPage, so the comma renders immediately after the linked text while preserving the intended spacing after the comma.services/diagramPrompt.ts-13-15 (1)
13-15: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrompt sentence is ungrammatical. "If an equilibrium point E is at (50, 50), ensuring the Supply Curve..." has no main clause; use "ensure".
✏️ Proposed fix
- - If an equilibrium point E is at (50, 50), ensuring the Supply Curve, Demand Curve, and any Shaded Regions ALL use the exact coordinate (50, 50). + - If an equilibrium point E is at (50, 50), ensure the Supply Curve, Demand Curve, and any Shaded Regions ALL use the exact coordinate (50, 50).🤖 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 `@services/diagramPrompt.ts` around lines 13 - 15, Correct the “Shared Coordinates (CRITICAL)” prompt text in diagramPrompt.ts by changing the ungrammatical “If an equilibrium point E is at (50, 50), ensuring…” sentence to use “ensure” as the main instruction, while preserving its coordinate-matching requirements.api/generate.ts-156-183 (1)
156-183: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a short timeout around the model call. The upstream API has no bound here, so a hanging response is only covered by the platform timeout and bypasses the existing refund path; abort after a short deadline so failed generations do not consume a counted credit.
🤖 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 `@api/generate.ts` around lines 156 - 183, Update the model call in the generate handler’s try/catch to use a short, explicit timeout with an abort signal, ensuring a hanging ai.models.generateContent request rejects and reaches the existing refund path. Preserve the current error logging, refund_ai_usage call, and 502 response behavior for timeout failures.services/keyObfuscation.ts-7-9 (1)
7-9: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
btoathrows on non-Latin1 input. A pasted key containing any character above U+00FF (stray unicode from a copy/paste) raisesInvalidCharacterError. The read side is wrapped intry/catchby callers, but the save side isn't (services/openrouter.tsLine 12,services/gemini.tsLine 14), so the settings save flow throws instead of reporting a bad key.🛡️ Proposed hardening
export function obfuscateKey(key: string): string { - return OBFUSCATION_PREFIX + btoa(key); + // Encode to UTF-8 first so non-Latin1 characters don't throw. + return OBFUSCATION_PREFIX + btoa(String.fromCharCode(...new TextEncoder().encode(key))); } export function deobfuscateKey(stored: string): string { if (!stored.startsWith(OBFUSCATION_PREFIX)) return stored; - return atob(stored.slice(OBFUSCATION_PREFIX.length)); + const bin = atob(stored.slice(OBFUSCATION_PREFIX.length)); + return new TextDecoder().decode(Uint8Array.from(bin, (c) => c.charCodeAt(0))); }Note this changes the encoding of newly stored keys; existing ASCII-only values decode identically, so no migration is needed.
🤖 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 `@services/keyObfuscation.ts` around lines 7 - 9, Update obfuscateKey to encode the key as UTF-8 before passing it to btoa, preventing InvalidCharacterError for Unicode input. Update the corresponding decode path to reverse the UTF-8 encoding, while preserving identical decoding for existing ASCII-only stored keys.api/delete-account.ts-10-14 (1)
10-14: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
ON DELETE CASCADEfromgraphstograph_versions.
graph_versions.graph_idis non-null but has no FK/cascade, andgraphs.user_idonly cascades toprofiles. Deleting a user removesgraphs, but leaves existinggraph_versionsrows and theircreated_atindex entries behind, so the deletion path does not fully match the docstring’s cascade guarantee.🤖 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 `@api/delete-account.ts` around lines 10 - 14, Update the database schema migration for graph_versions.graph_id to add a foreign key referencing graphs with ON DELETE CASCADE. Preserve the non-null constraint and ensure existing graph_versions rows and their related indexes are removed when a graph is deleted, matching the cascade behavior described in the delete-account documentation.
🧹 Nitpick comments (15)
services/useCloudSync.ts (1)
87-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winTrack the re-run timeout so cleanup can cancel it.
The 500 ms re-run timer isn't stored in a ref, so the unmount cleanup at Line 140 can't clear it; a sync can still fire after unmount/sign-out. Reusing
timerRef(or a second ref) makes it cancellable.♻️ Proposed change
} finally { runningRef.current = false; if (rerunRef.current) { rerunRef.current = false; - window.setTimeout(() => { void runSync(); }, 500); + if (timerRef.current) window.clearTimeout(timerRef.current); + timerRef.current = window.setTimeout(() => { + timerRef.current = null; + void runSync(); + }, 500); } }🤖 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 `@services/useCloudSync.ts` around lines 87 - 93, Store the 500 ms timeout created in the finally block of runSync in the existing timerRef (or a dedicated timeout ref), and have the unmount/sign-out cleanup clear that stored timer before resetting it. Keep the rerunRef behavior unchanged while ensuring runSync cannot be triggered by this delayed callback after cleanup.components/ComponentLibrary.tsx (1)
70-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard against out-of-order/stale template fetches.
fetchCustomTemplatesresolves asynchronously; ifuser/isOpen/isProchanges before it lands, a stale response can overwrite the newer list (including the signed-out[]). A cancellation flag in the effect avoids it.♻️ Proposed change
useEffect(() => { if (!user) { setCustomTemplates([]); return; } setCustomTemplates(listCachedTemplates(user.id)); - if (isOpen && isPro) { - fetchCustomTemplates(user.id).then(setCustomTemplates); - } + if (!isOpen || !isPro) return; + let cancelled = false; + fetchCustomTemplates(user.id).then((t) => { if (!cancelled) setCustomTemplates(t); }); + return () => { cancelled = true; }; }, [isOpen, user, isPro]);🤖 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 `@components/ComponentLibrary.tsx` around lines 70 - 79, Add a per-effect cancellation flag in the useEffect that loads custom templates, set it during cleanup, and only apply fetchCustomTemplates results while the effect is still active. Preserve the immediate signed-out reset and cached-template behavior, ensuring stale responses cannot overwrite newer state.components/ShareModal.tsx (1)
33-43: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBoth modals key their load effect on the
graphobject instead ofgraph.id.activeGraphinApp.tsxis auseMemoderived fromgraphs, so its identity changes on every autosave; while either modal is open, ordinary diagram editing re-runs these effects and re-issues Supabase queries.
components/ShareModal.tsx#L33-L43: replacegraphwithgraph?.idin the effect body and dependency array.components/CloudHistoryModal.tsx#L33-L41: replacegraphwithgraph?.idin the effect body and dependency array, and resetversionswhen the id changes.🤖 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 `@components/ShareModal.tsx` around lines 33 - 43, Key both modal load effects on the graph ID rather than the changing graph object. In components/ShareModal.tsx lines 33-43, use graph?.id in the effect logic and dependency array; in components/CloudHistoryModal.tsx lines 33-41, do the same and reset versions when the graph ID changes.components/ComparePage.tsx (1)
147-191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComparison table needs row headers and a keyboard-scrollable container.
Row labels are plain
<td>, so assistive tech can't associate a cell with its row; and theoverflow-x-autowrapper around amin-w-[760px]table can't be scrolled by keyboard alone because it isn't focusable.♿ Proposed change
- <div className="overflow-x-auto rounded-2xl border border-slate-200 shadow-sm"> + <div + className="overflow-x-auto rounded-2xl border border-slate-200 shadow-sm" + tabIndex={0} + role="region" + aria-label="Feature comparison" + > @@ - <th className="p-4 font-semibold text-gray-500 w-[22%]"></th> + <th scope="col" className="p-4 font-semibold text-gray-500 w-[22%]"> + <span className="sr-only">Feature</span> + </th> @@ - <td className="p-4 font-medium text-gray-700">{row.label}</td> + <th scope="row" className="p-4 font-medium text-gray-700 text-left">{row.label}</th>(the remaining
<th>s in<thead>should also getscope="col".)🤖 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 `@components/ComparePage.tsx` around lines 147 - 191, Update the comparison table in the ROWS mapping to render each row label as a row header with scope="row", and add scope="col" to every header cell in the table header. Make the overflow-x-auto wrapper keyboard-focusable, preserving its horizontal scrolling behavior for keyboard users.components/LandingPage.tsx (1)
750-761: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRoute Privacy/Terms through the SPA callback contract.
The Vercel rewrite covers
/privacyand/terms, but these hard links still reload the page while the adjacent footer buttons useonOpenPricing/onOpenCompare. PassonOpenPrivacy/onOpenTermsprops toLandingPagefor consistent SPA navigation.🤖 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 `@components/LandingPage.tsx` around lines 750 - 761, Update the LandingPage footer Privacy and Terms links to use the SPA callback contract instead of hard-coded href navigation: accept onOpenPrivacy and onOpenTerms props in LandingPage, then invoke the corresponding callbacks from those links while preserving their existing styling and labels. Ensure the parent passes both callbacks into LandingPage.services/billing.ts (1)
3-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on network requests.
callBillingEndpointanddeleteAccounthave no request timeout; a hung response leaves the caller's loading UI stuck indefinitely. See consolidated comment withservices/hostedAi.tsfor a shared fix.Also applies to: 40-56
🤖 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 `@services/billing.ts` around lines 3 - 24, Update callBillingEndpoint and deleteAccount to enforce a finite timeout on their network requests, using the shared timeout approach referenced for services/hostedAi.ts. Ensure timed-out requests abort and flow through the existing error handling so callers do not remain loading indefinitely.services/hostedAi.ts (2)
21-33: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing request timeout on hosted AI calls.
Neither
generateDiagramDataHostednorfetchHostedUsageset a timeout/AbortController. See the consolidated comment for a shared fix across this file andservices/billing.ts.Also applies to: 52-54
🤖 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 `@services/hostedAi.ts` around lines 21 - 33, Add a request timeout using an AbortController to both generateDiagramDataHosted and fetchHostedUsage, passing its signal to each fetch call and aborting after the established timeout interval. Ensure timers are cleaned up when requests complete, while preserving the existing connection-error handling.
1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd request timeouts to first-party API
fetch()calls. None of the client-side wrappers around/api/generate,/api/usage,/api/checkout,/api/portal, or/api/delete-accountset a timeout, so a hung or very slow server response leaves the caller's loading UI (spinner, "Generating…", checkout/portal buttons) stuck indefinitely with no user recourse short of a page reload.
services/hostedAi.ts#L21-33: wrap the/api/generatefetchwith anAbortControllertimeout (AI generation is the longest-running, highest-visibility call).services/hostedAi.ts#L52-54: wrap the/api/usagefetchwith the same timeout helper.services/billing.ts#L3-24: wrapcallBillingEndpoint'sfetchwith the same timeout helper.services/billing.ts#L40-56: wrapdeleteAccount'sfetchwith the same timeout helper.🤖 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 `@services/hostedAi.ts` at line 1, Add a shared AbortController-based timeout helper and use it for the fetch calls in hostedAi generation and usage flows, plus billing’s callBillingEndpoint and deleteAccount. Ensure each request aborts after the configured timeout while preserving existing request handling and error propagation.services/sync.ts (1)
264-267: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftEvery debounced sync round-trips full diagram payloads for all graphs/projects, even when nothing changed remotely.
The
selectat Lines 265-266 pulls the fulldataJSON blob for every graph/project the user owns on everysyncCloud()call, not just the ones that actually changed. Given this runs on a "debounced sync path" (per the comment at Line 454) potentially on every edit, this means repeatedly transferring the full diagram content for a user's entire library even when onlylast_modifiedneeds checking for most rows.Consider a two-phase approach: first select only
id, last_modified, deletedto diff against local state, then fetch fulldataonly for the rows that actually need to be pulled.🤖 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 `@services/sync.ts` around lines 264 - 267, Update the syncCloud data-fetch flow around the graphRes and projectRes queries to use two phases: initially select only id, last_modified, and deleted for remote diffing, then fetch the full metadata and data payload only for rows determined to require pulling. Preserve existing handling for unchanged, deleted, and locally modified records while avoiding full data transfers on every debounced sync round.api/generate.ts (1)
40-72: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider hoisting the client out of the request path.
resolveAiClient()constructs a newGoogleGenAI(and re-parsesGOOGLE_SERVICE_ACCOUNT_JSON) on every invocation; env is static per instance, so a module-level memo saves per-request work and lets ADC token caching survive across warm invocations.🤖 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 `@api/generate.ts` around lines 40 - 72, Hoist the resolved AI client out of the request path by memoizing the result of resolveAiClient() at module scope. Ensure GOOGLE_SERVICE_ACCOUNT_JSON is parsed and GoogleGenAI is constructed once per instance, while preserving the existing environment-priority selection and null behavior; reuse the cached client wherever resolveAiClient() is currently invoked.package.json (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
dev:apidepends on an undeclaredvercelCLI. Contributors without a global install get a "command not found". Either addverceltodevDependenciesor document the prerequisite in the setup docs.🤖 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 `@package.json` at line 39, Add the Vercel CLI package to package.json's devDependencies so the existing dev:api script can run without requiring a global installation. Use the project's existing dependency versioning conventions and avoid changing the script itself.supabase/schema.sql (1)
458-464: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueInsert branch ignores
p_limit. Withp_limit <= 0(e.g. quota disabled via config) the first generation each month still succeeds. Addwhere p_limit > 0semantics if a zero limit should ever be meaningful.🤖 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 `@supabase/schema.sql` around lines 458 - 464, The ai_usage upsert does not enforce p_limit on the initial insert. Update the insert/upsert flow around the ai_usage conflict handler so p_limit <= 0 prevents inserting a first monthly usage record and follows the existing quota-exceeded behavior, while preserving the current increment guard for existing records..github/workflows/db-keepalive.yml (2)
37-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd curl timeouts. Without
--max-time, a hung connection stalls the job until the 6h default timeout.♻️ Proposed change
code=$(curl -s -o /dev/null -w '%{http_code}' \ + --connect-timeout 10 --max-time 30 \ "$SUPABASE_URL/rest/v1/profiles?select=id&limit=1" \🤖 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 @.github/workflows/db-keepalive.yml around lines 37 - 40, Update the curl invocation in the keepalive workflow to include a finite maximum request duration using curl’s timeout option, ensuring a hung Supabase request cannot stall the job for the workflow’s full timeout period while preserving the existing URL, headers, and HTTP-status capture.
12-16: 🩺 Stability & Availability | 🔵 TrivialKeepalive depends on scheduled workflows staying enabled. GitHub disables
scheduletriggers in repositories with no activity for 60 days, and scheduled runs can be delayed or dropped under load — either silently defeats the 7-day pause guard. Consider a redundant external ping (e.g. an uptime/cron service hitting the same PostgREST URL) or alerting on missed runs.🤖 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 @.github/workflows/db-keepalive.yml around lines 12 - 16, Augment the scheduled keepalive defined by the workflow’s on.schedule configuration with a redundant external ping or equivalent missed-run alert, targeting the same PostgREST keepalive endpoint. Ensure the fallback preserves the requirement that the database is contacted within every seven-day window even if GitHub disables or delays scheduled workflow runs.api/portal.ts (1)
26-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConfig errors are indistinguishable from "no billing account" here.
If
getPolar()throws becausePOLAR_ACCESS_TOKENisn't set, this returns the same 404 "No billing account found... wait a few seconds and try again" as a genuine missing-customer case — misleading during an actual misconfiguration/outage, and inconsistent with the 503 "not configured on this deployment" pattern used elsewhere in this PR (checkout.ts, delete-account.ts, usage.ts).♻️ Proposed fix
let user; try { user = await getUserFromRequest(req); } catch (err) { console.error('portal: auth backend error', err); return res.status(503).json({ error: 'Account service is not configured on this deployment.' }); } if (!user) { return res.status(401).json({ error: 'Please sign in first.' }); } + + let polar; + try { + polar = getPolar(); + } catch (err) { + console.error('portal: billing backend not configured', err); + return res.status(503).json({ error: 'Billing is not configured on this deployment.' }); + } try { - const polar = getPolar(); const session = await polar.customerSessions.create({ externalCustomerId: user.id, }); return res.status(200).json({ url: session.customerPortalUrl }); } catch (err) { console.error('portal: failed to create customer session', err); return res.status(404).json({ error: 'No billing account found. If you just subscribed, wait a few seconds and try again.', }); }🤖 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 `@api/portal.ts` around lines 26 - 37, Update the error handling around getPolar and polar.customerSessions.create so configuration failures such as a missing POLAR_ACCESS_TOKEN return the established 503 “not configured on this deployment” response, while genuine missing-customer errors retain the existing 404 response. Follow the existing handling pattern used by checkout.ts, delete-account.ts, or usage.ts and keep the portal success response unchanged.
🤖 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 @.github/workflows/update-supporters.yml:
- Around line 25-46: Update the checkout step to set persist-credentials to
false, then configure authentication immediately before the README commit/push
commands in “Commit if the README changed” using the workflow token. Keep npm ci
running without persisted Git credentials and preserve the existing conditional
commit behavior.
In `@api/generate.ts`:
- Around line 124-133: Update the profile lookup and entitlement flow around
getProfile and isProfilePro so a failed profile request is handled separately
from a successfully loaded non-Pro profile. Preserve the existing 402 not_pro
response only when profile data confirms the user lacks entitlement; for lookup
errors, return an appropriate server-error response instead of treating the user
as unauthorized.
- Around line 139-154: Update the quota check following the increment_ai_usage
RPC in the generate handler to fail closed when newCount is not a number. Reject
unexpected null or string results with the existing server-error response path,
while preserving the 429 quota-exceeded response for numeric values below zero.
In `@api/webhooks/polar.ts`:
- Line 22: Move the entitled Polar status set into services/entitlement.ts as
the shared ENTITLED_POLAR_STATUSES constant, then import and use it in
api/webhooks/polar.ts, api/checkout.ts, and api/delete-account.ts instead of
defining local status sets, preserving the existing active, trialing, and
past_due values.
- Around line 56-110: The read-compute-write flow in applySubscriptionState is
vulnerable to concurrent same-subscription webhook updates overwriting each
other. Make the profile state transition atomic by moving the current-state
read, entitlement calculation, and update into a transactional/Postgres function
that locks the user row with SELECT ... FOR UPDATE, or condition the existing
update on the previously-read pro_until and polar_subscription_id and retry when
no row is updated; preserve the existing stale-subscription and non-decreasing
proUntil guards.
In `@App.tsx`:
- Around line 254-273: Update the cross-account guard in the useEffect keyed by
user?.id and hasInitialized so switching accounts no longer clears or overwrites
the previous user's local-only graphs and projects. Namespace persisted local
data by owner, including an anonymous bucket, and load/save the active user's
namespace when the owner changes; preserve each account's data for later return
instead of replacing it with empty arrays. Do not rely on the current global
STORAGE_KEYS.owner flow as the sole isolation mechanism.
In `@components/LegalPages.tsx`:
- Around line 131-137: Update the child-account handling associated with the
“Children & students” section to require verifiable parental consent before
collecting data from users under the applicable age, or block account creation
for those minors through an age-gating flow. Ensure signup paths for
email/password and Google accounts enforce this outcome, and revise the policy
text to accurately describe the implemented behavior after legal review.
- Around line 79-81: Update the hosted AI prompts disclosure in LegalPages to
match the configured backend behavior: either enforce Vertex AI
zero-data-retention and disable logging/tracking before every
ai.models.generateContent call across VERTEX_API_KEY/GOOGLE_CLOUD_PROJECT paths,
or replace the blanket no-training statement with provider-specific prompt
handling disclosures.
- Around line 205-210: Update the hosted AI legal text in the “Hosted AI & fair
use” section of LegalPages to replace the “unlimited generations” BYOK promise
with wording that clarifies BYOK is not metered by this app but remains subject
to the provider’s limits, usage rules, and costs.
In `@services/shares.ts`:
- Around line 63-85: Update getShareIdForGraph and getShareIdForProject to
inspect and propagate Supabase lookup errors instead of returning null on
failures; preserve null only when the query succeeds with no matching share.
Adjust createOrUpdateGraphShare and the corresponding project-share flow to
catch the propagated error and return { error: friendlyShareError(...) } without
minting or upserting a new slug.
- Around line 125-129: Update revokeShare to request the deleted share row from
the shares delete operation and treat an empty returned result as an error.
Preserve the existing Supabase-unavailable and database-error handling, and only
return success when the targeted share is actually deleted so ShareModal does
not clear state for an unresolved revoke.
In `@services/sync.ts`:
- Around line 262-263: Update syncCloud() to remove tombstone entries whose ids
are present in finalGraphs or finalProjects immediately after loadTombstones()
and before generating tombstone rows. Preserve tombstones for ids absent from
both collections so the catch-all phase still queues genuinely deleted records.
In `@supabase/schema.sql`:
- Around line 293-313: Update public.purge_versions_for_deleted_graph so its
condition handles INSERT events without accessing OLD: treat inserts with
new.deleted = true as eligible, while retaining the existing transition check
for UPDATE events. Preserve the deletion of matching graph_versions and the
trigger configuration.
---
Outside diff comments:
In `@App.tsx`:
- Around line 690-721: Update handleImportData so fetchCloudIds() runs before
setGraphs and setProjects, then record tombstones for cloud-only graph and
project IDs using the imported ID sets. Remove the trailing fetchCloudIds block
after the state updates, preserving the existing best-effort null handling.
In `@components/SettingsPage.tsx`:
- Around line 394-404: Update the provider initialization in SettingsPage so a
persisted "hosted" value is replaced with a supported BYOK provider when
cloudConfigured is false. Ensure this fallback runs on mount or when
configuration is loaded, while preserving the existing provider selection when
hosted is available and keeping the select, key fields, and model sections
synchronized.
---
Minor comments:
In `@api/_lib/polar.ts`:
- Around line 32-40: Update the origin-selection logic around the origin header
and fallback URL so payment redirects never use an arbitrary caller-controlled
Origin. Require the configured APP_URL (or an equivalent fixed allowlist) for
non-local environments, while preserving direct localhost/loopback handling for
explicit local development; ensure authenticated checkout cannot fall back to an
untrusted request origin.
In `@api/delete-account.ts`:
- Around line 10-14: Update the database schema migration for
graph_versions.graph_id to add a foreign key referencing graphs with ON DELETE
CASCADE. Preserve the non-null constraint and ensure existing graph_versions
rows and their related indexes are removed when a graph is deleted, matching the
cascade behavior described in the delete-account documentation.
In `@api/generate.ts`:
- Around line 156-183: Update the model call in the generate handler’s try/catch
to use a short, explicit timeout with an abort signal, ensuring a hanging
ai.models.generateContent request rejects and reaches the existing refund path.
Preserve the current error logging, refund_ai_usage call, and 502 response
behavior for timeout failures.
In `@App.tsx`:
- Around line 822-828: Replace the title-based userNamed inference in the
generation flow with an explicit flag recorded by renameGraph. Set that flag
only when the user renames the graph, and use it when deciding whether to
preserve activeGraph.title, so AI-generated titles remain updateable until an
explicit rename occurs.
In `@components/AccountSection.tsx`:
- Around line 82-106: The checkout poll attempt counter in the useEffect must
persist when refreshProfile changes and the effect restarts. Move attempts to a
useRef (or equivalent stable state), reset it when a new checkout begins,
increment the stable value during polling, and use it for the delayed fallback
while preserving the existing cleanup behavior.
In `@components/AuthModal.tsx`:
- Around line 165-196: Add accessible names to both email and password inputs in
the AuthModal form, and to the reset-form email input near the reset flow. Use
clear aria-labels or existing visually hidden labels, while preserving the
current input behavior and autocomplete settings.
In `@components/ComponentLibrary.tsx`:
- Around line 247-271: Update the custom template rows rendered in
filteredCustom.map to be keyboard accessible: add button semantics, make each
row focusable, and handle Enter and Space by invoking addCustomTemplate(t),
while preserving the nested delete button’s stopPropagation behavior.
- Line 213: Update handleSaveTemplate in ComponentLibrary so it immediately
returns when a save is already in progress or the template name is empty,
ensuring both button clicks and the Enter-key onKeyDown path share the same
guard and cannot create duplicate or blank templates.
In `@components/LegalPages.tsx`:
- Around line 37-39: Update the Last updated paragraph and footer in the
LegalPages component to use text-gray-500 or a darker text color instead of
text-gray-400, preserving the existing layout and other styling.
In `@components/SharedViewPage.tsx`:
- Around line 168-174: Remove the whitespace expression following the “IB
EconGraph AI” button in the footer of SharedViewPage, so the comma renders
immediately after the linked text while preserving the intended spacing after
the comma.
In `@docs/BACKEND_SETUP.md`:
- Around line 163-165: Reconcile the grace-period value described near the
webhook entitlement documentation with the implemented billing behavior and the
corresponding CHANGELOG entry. Update the conflicting documentation so the
stated grace period is consistent across both references, preserving the
existing entitlement and renewal semantics.
- Around line 101-105: Update the environment-variable code fences in
BACKEND_SETUP.md, including the blocks around VERTEX_API_KEY and the additional
referenced blocks, to specify the dotenv language. Preserve their existing
contents and formatting while ensuring every affected fence is marked for dotenv
syntax.
In `@scripts/seo-content.mjs`:
- Around line 454-458: Update the licensing statement in the faq array of
seo-content.mjs to reference AGPL-3.0 instead of MIT, keeping the rest of the
classroom-use answer unchanged.
In `@services/diagramPrompt.ts`:
- Around line 13-15: Correct the “Shared Coordinates (CRITICAL)” prompt text in
diagramPrompt.ts by changing the ungrammatical “If an equilibrium point E is at
(50, 50), ensuring…” sentence to use “ensure” as the main instruction, while
preserving its coordinate-matching requirements.
In `@services/keyObfuscation.ts`:
- Around line 7-9: Update obfuscateKey to encode the key as UTF-8 before passing
it to btoa, preventing InvalidCharacterError for Unicode input. Update the
corresponding decode path to reverse the UTF-8 encoding, while preserving
identical decoding for existing ASCII-only stored keys.
---
Nitpick comments:
In @.github/workflows/db-keepalive.yml:
- Around line 37-40: Update the curl invocation in the keepalive workflow to
include a finite maximum request duration using curl’s timeout option, ensuring
a hung Supabase request cannot stall the job for the workflow’s full timeout
period while preserving the existing URL, headers, and HTTP-status capture.
- Around line 12-16: Augment the scheduled keepalive defined by the workflow’s
on.schedule configuration with a redundant external ping or equivalent
missed-run alert, targeting the same PostgREST keepalive endpoint. Ensure the
fallback preserves the requirement that the database is contacted within every
seven-day window even if GitHub disables or delays scheduled workflow runs.
In `@api/generate.ts`:
- Around line 40-72: Hoist the resolved AI client out of the request path by
memoizing the result of resolveAiClient() at module scope. Ensure
GOOGLE_SERVICE_ACCOUNT_JSON is parsed and GoogleGenAI is constructed once per
instance, while preserving the existing environment-priority selection and null
behavior; reuse the cached client wherever resolveAiClient() is currently
invoked.
In `@api/portal.ts`:
- Around line 26-37: Update the error handling around getPolar and
polar.customerSessions.create so configuration failures such as a missing
POLAR_ACCESS_TOKEN return the established 503 “not configured on this
deployment” response, while genuine missing-customer errors retain the existing
404 response. Follow the existing handling pattern used by checkout.ts,
delete-account.ts, or usage.ts and keep the portal success response unchanged.
In `@components/ComparePage.tsx`:
- Around line 147-191: Update the comparison table in the ROWS mapping to render
each row label as a row header with scope="row", and add scope="col" to every
header cell in the table header. Make the overflow-x-auto wrapper
keyboard-focusable, preserving its horizontal scrolling behavior for keyboard
users.
In `@components/ComponentLibrary.tsx`:
- Around line 70-79: Add a per-effect cancellation flag in the useEffect that
loads custom templates, set it during cleanup, and only apply
fetchCustomTemplates results while the effect is still active. Preserve the
immediate signed-out reset and cached-template behavior, ensuring stale
responses cannot overwrite newer state.
In `@components/LandingPage.tsx`:
- Around line 750-761: Update the LandingPage footer Privacy and Terms links to
use the SPA callback contract instead of hard-coded href navigation: accept
onOpenPrivacy and onOpenTerms props in LandingPage, then invoke the
corresponding callbacks from those links while preserving their existing styling
and labels. Ensure the parent passes both callbacks into LandingPage.
In `@components/ShareModal.tsx`:
- Around line 33-43: Key both modal load effects on the graph ID rather than the
changing graph object. In components/ShareModal.tsx lines 33-43, use graph?.id
in the effect logic and dependency array; in components/CloudHistoryModal.tsx
lines 33-41, do the same and reset versions when the graph ID changes.
In `@package.json`:
- Line 39: Add the Vercel CLI package to package.json's devDependencies so the
existing dev:api script can run without requiring a global installation. Use the
project's existing dependency versioning conventions and avoid changing the
script itself.
In `@services/billing.ts`:
- Around line 3-24: Update callBillingEndpoint and deleteAccount to enforce a
finite timeout on their network requests, using the shared timeout approach
referenced for services/hostedAi.ts. Ensure timed-out requests abort and flow
through the existing error handling so callers do not remain loading
indefinitely.
In `@services/hostedAi.ts`:
- Around line 21-33: Add a request timeout using an AbortController to both
generateDiagramDataHosted and fetchHostedUsage, passing its signal to each fetch
call and aborting after the established timeout interval. Ensure timers are
cleaned up when requests complete, while preserving the existing
connection-error handling.
- Line 1: Add a shared AbortController-based timeout helper and use it for the
fetch calls in hostedAi generation and usage flows, plus billing’s
callBillingEndpoint and deleteAccount. Ensure each request aborts after the
configured timeout while preserving existing request handling and error
propagation.
In `@services/sync.ts`:
- Around line 264-267: Update the syncCloud data-fetch flow around the graphRes
and projectRes queries to use two phases: initially select only id,
last_modified, and deleted for remote diffing, then fetch the full metadata and
data payload only for rows determined to require pulling. Preserve existing
handling for unchanged, deleted, and locally modified records while avoiding
full data transfers on every debounced sync round.
In `@services/useCloudSync.ts`:
- Around line 87-93: Store the 500 ms timeout created in the finally block of
runSync in the existing timerRef (or a dedicated timeout ref), and have the
unmount/sign-out cleanup clear that stored timer before resetting it. Keep the
rerunRef behavior unchanged while ensuring runSync cannot be triggered by this
delayed callback after cleanup.
In `@supabase/schema.sql`:
- Around line 458-464: The ai_usage upsert does not enforce p_limit on the
initial insert. Update the insert/upsert flow around the ai_usage conflict
handler so p_limit <= 0 prevents inserting a first monthly usage record and
follows the existing quota-exceeded behavior, while preserving the current
increment guard for existing records.
🪄 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: 9c716cb2-5de2-4056-a24a-c2e9b6b87059
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
.env.example.github/workflows/db-keepalive.yml.github/workflows/update-supporters.yml.gitignoreApp.tsxCHANGELOG.mdREADME.mdapi/_lib/polar.tsapi/_lib/supabaseAdmin.tsapi/checkout.tsapi/delete-account.tsapi/generate.tsapi/portal.tsapi/usage.tsapi/webhooks/polar.tscomponents/AccountSection.tsxcomponents/AuthModal.tsxcomponents/CloudHistoryModal.tsxcomponents/ComparePage.tsxcomponents/ComponentLibrary.tsxcomponents/LandingPage.tsxcomponents/LegalPages.tsxcomponents/PricingPage.tsxcomponents/SettingsPage.tsxcomponents/ShareModal.tsxcomponents/SharedViewPage.tsxdocs/BACKEND_SETUP.mdindex.htmlindex.tsxpackage.jsonpublic/sitemap.xmlscripts/generate-seo-pages.mjsscripts/seo-content.mjsscripts/update-supporters.mjsservices/ai.tsservices/aiProvider.tsservices/auth.tsxservices/billing.tsservices/cloudErrors.tsservices/customTemplates.tsservices/diagramPrompt.tsservices/entitlement.tsservices/gemini.tsservices/hostedAi.tsservices/keyObfuscation.tsservices/openrouter.tsservices/shares.tsservices/supabaseClient.tsservices/sync.tsservices/useCloudSync.tssupabase/schema.sqlvercel.jsonvite-env.d.tsvite.config.ts
💤 Files with no reviewable changes (1)
- public/sitemap.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 55 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/SettingsPage.tsx">
<violation number="1" location="components/SettingsPage.tsx:402">
P2: Removing cloud configuration can leave a persisted `hosted` provider active even though this option is no longer rendered. Reset or fall back to a BYOK provider when cloud is unavailable so zero-config deployments do not block AI generation for users with existing local storage.</violation>
</file>
<file name="services/diagramPrompt.ts">
<violation number="1" location="services/diagramPrompt.ts:121">
P2: The new shared module services/diagramPrompt.ts exports DIAGRAM_SYSTEM_INSTRUCTION and buildHistoryContext as the single source of truth for all AI providers, but services/openrouter.ts still maintains its own identical inline copies of both. This creates a drift risk: any prompt or formatting update to the shared module will silently leave OpenRouter behind. Import the shared exports in services/openrouter.ts and remove the inline duplicates.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Licensing (the project is AGPL-3.0, two places still claimed MIT): - package.json: license MIT -> AGPL-3.0-or-later - scripts/seo-content.mjs: the PPC landing page told visitors the project was MIT open source, contradicting its own footer Correctness / security: - api/generate.ts: a failed profile lookup returned 402 not_pro, telling a paying Supporter their plan had lapsed during a transient DB blip. Return 503 instead. - api/generate.ts: the quota gate only tripped on `typeof newCount === 'number'`, so an unexpected RPC return type silently skipped metering and handed out unlimited generations on the hosted key. Fail closed. - api/usage.ts: the ai_usage query error was ignored, reporting `used: 0` on failure and showing a full quota to someone who had spent it. - api/webhooks/polar.ts: the profile SELECT error was ignored, so a failed read looked like "nothing on file" and could clobber the live subscription. Throw so the handler answers 500 and Polar retries. - api/delete-account.ts: cancellation was gated on our own pro_status, so a stale value skipped it and left a subscription billing a deleted account. Always attempt it when a subscription id is on file. Also, any Polar lookup failure was read as "already gone"; only a 404 proves that now. - services/shares.ts: the existing-share lookup discarded its error, so a transient failure read as "no share exists" and minted a second slug for the same content. Revoking the link shown in the UI then left the other one publicly readable. Surface the failure, and resolve a lost creation race to the winning link. - supabase/schema.sql: revoke a share when its graph or project is deleted. The client already prunes these during sync, but that pass is best-effort and swallows failures, leaving deleted diagrams publicly readable. - supabase/schema.sql: unique index for one share per graph/project, with a dedupe of any rows predating it so the migration applies to a live database. UI / client: - App.tsx: derive the preserved graph title from current state rather than the snapshot taken before the await, so renaming during generation still wins. - App.tsx: blank the canvas and undo stack on account switch. Clearing the collections alone left the previous account's diagram on screen until the first cloud pull landed. - components/LandingPage.tsx: drop `font-small`, not a Tailwind class. - services/hostedAi.ts: a failed session restore rejected instead of returning null, producing an unhandled rejection in the usage meter. Build / config: - scripts/generate-seo-pages.mjs: a trailing `_` or `^` in a label hung the build forever. The scan could not advance past the marker, so the outer loop never progressed. - vite.config.ts: drop the `define` entries that inlined GEMINI_API_KEY into the client bundle. Nothing referenced them, but any future code that did would have shipped the server key to the browser. Schema changes verified against a throwaway Postgres: applies cleanly, is idempotent, collapses pre-existing duplicate shares, and the delete triggers and unique indexes behave.
…g npm ci actions/checkout leaves a contents:write token in .git/config, where any dependency install script run by `npm ci` could read it. Check out without persisted credentials and pass the token explicitly on the push instead.
There was a problem hiding this comment.
All reported issues were addressed across 55 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Local diagrams and projects lived under one set of keys shared by everyone using the browser, with an `econgraph_owner` marker naming who they belonged to. Signing in as a different account deleted them. For Supporters that was survivable (their copy is in the cloud) but for free accounts and signed-out work it was permanent, silent data loss on any shared computer. Each account now gets its own namespace, plus one shared "guest" namespace for work done signed out. Switching accounts swaps which namespace is live and never deletes the other, so signing out and back in returns you to exactly what you left. Signed-out work still follows you into an account, but only when that cannot mix two people's diagrams together: the account must have nothing of its own, and for Supporters only once the first cloud pull has answered whether the account is really empty. If the account already has diagrams, the signed-out work stays where it is and is there again on sign out. That rule is `decideGuestAdoption`, kept as a pure function so it can be tested directly. Guest keeps the original key names, so existing local work needs no migration. Data that belonged to an account (per `econgraph_owner`) is moved into that account's namespace once, on first run under the new scheme. Also closes three ways data could still cross between accounts: - sync results that arrive after an account switch are dropped, instead of importing the previous account's cloud data into whoever is signed in now - sync is withheld until the signed-in account's own data is the data in memory, so a switch can't upload the outgoing account's diagrams - writes are suppressed while a namespace swap is in flight And two editor bugs this made reachable: - the auto-open effect selected a newly created graph unconditionally, even when its own guard discarded it, leaving a selected id that matched nothing - a Supporter's first render counted as "not awaiting the first pull", so an empty store briefly looked real and produced a throwaway blank diagram Verified: 23 store tests and 19 account-flow scenario tests (adoption, segmentation, sign-out/in, two free accounts, late sync after a switch), plus the real migration observed running against a live signed-in profile, which moved 12KB of existing diagrams into the correct namespace with nothing lost.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
App.tsx (1)
271-336: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winScope-switch effect resets
historyIndexstate but nothistoryIndexRef, enabling an out-of-bounds undo crash.On an account switch,
historyRef.currentis reset to a 1-item array ([blank]) andhistoryIndexstate goes to 0, buthistoryIndexRef.current(used byundo/redo) is left untouched. If it was left> 0from the previous scope's session, pressing Ctrl+Z right after the switch (before any edit or graph open) passesundo()'shistoryIndexRef.current > 0guard, then indexeshistoryRef.current[nextIndex]past the new array's bounds, returningundefinedintosetCurrentDiagram. That crashesDiagramRendereronce it accesses fields on the diagram. The keyboard shortcut for undo (line 565-568) callsundo()unconditionally, with nocanUndo/state-based guard, so this path is directly reachable.🐛 Proposed fix
setActiveGraphId(null); const blank = { ...EMPTY_DIAGRAM }; setCurrentDiagram(blank); setHistory([blank]); historyRef.current = [blank]; setHistoryIndex(0); + historyIndexRef.current = 0; setLoadedScope(storeScope); setHasInitialized(true); }, [storeScope, loadedScope]);🤖 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 `@App.tsx` around lines 271 - 336, Update the scope-switch reset logic in the storeScope effect to also set historyIndexRef.current to 0 alongside historyRef.current, historyIndex, and the blank diagram. Ensure undo and redo observe the new one-item history immediately after switching accounts and cannot index beyond the reset history.vite.config.ts (1)
6-84: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winTighten dev API route containment before module resolution.
/api/../../../../secret.tsresolves to absolute paths outsideapi/, including/secret.ts.ts, becauserelstill contains..andpath.join()can escape through that segment. Normalize the extracted route, reject../consecutive slashes before building variants, and only allow segments underapi/.🤖 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 `@vite.config.ts` around lines 6 - 84, Harden devApiPlugin’s route extraction before constructing variants: normalize the pathname, reject traversal segments, empty/consecutive segments, and any route that resolves outside the root api directory. Apply this validation before fs.existsSync or ssrLoadModule, returning a 404 for invalid routes while preserving valid file and index route resolution.
🧹 Nitpick comments (1)
App.tsx (1)
187-189: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRef mutated directly during render (
loadedScopeRef,graphsRef).Both
loadedScopeRef.current = loadedScope;(line 189) andgraphsRef.current = graphs;(line 799) assign to a ref in the component body instead of an effect. React can discard or replay a render pass, so these assignments can leak stale values from an aborted render. The file already handles this correctly elsewhere —activeGraphIdRef/currentDiagramRefare synced via dedicateduseEffects (lines 334-335) — making these two an inconsistency rather than a deliberate exception.♻️ Proposed fix
- const loadedScopeRef = useRef<string | null>(null); - loadedScopeRef.current = loadedScope; + const loadedScopeRef = useRef<string | null>(null); + useEffect(() => { loadedScopeRef.current = loadedScope; }, [loadedScope]);- const graphsRef = useRef(graphs); - graphsRef.current = graphs; + const graphsRef = useRef(graphs); + useEffect(() => { graphsRef.current = graphs; }, [graphs]);Also applies to: 798-800
🤖 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 `@App.tsx` around lines 187 - 189, Move the loadedScopeRef.current assignment out of the App component render body and synchronize it in a dedicated useEffect keyed to loadedScope, matching the existing activeGraphIdRef/currentDiagramRef pattern. Apply the same change to graphsRef.current, using an effect keyed to graphs, and remove both direct render-time mutations.Source: Linters/SAST tools
🤖 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 `@services/localStore.ts`:
- Around line 55-65: Update writeRaw to report localStorage failures to callers
instead of swallowing them after logging, and ensure the write/remove operation
is confirmed successful. In migrateLegacyStore and adoptScope, clear the source
namespace only after the destination write succeeds; preserve the source data
when any required write fails.
In `@supabase/schema.sql`:
- Around line 417-448: Update purge_shares_for_deleted_content() to guard OLD
access by checking TG_OP before evaluating old.deleted, so INSERT executions
only inspect NEW and return without deleting shares unless the operation is an
update transitioning deleted to true. Preserve the existing graph/project share
deletion branches and trigger definitions.
---
Outside diff comments:
In `@App.tsx`:
- Around line 271-336: Update the scope-switch reset logic in the storeScope
effect to also set historyIndexRef.current to 0 alongside historyRef.current,
historyIndex, and the blank diagram. Ensure undo and redo observe the new
one-item history immediately after switching accounts and cannot index beyond
the reset history.
In `@vite.config.ts`:
- Around line 6-84: Harden devApiPlugin’s route extraction before constructing
variants: normalize the pathname, reject traversal segments, empty/consecutive
segments, and any route that resolves outside the root api directory. Apply this
validation before fs.existsSync or ssrLoadModule, returning a 404 for invalid
routes while preserving valid file and index route resolution.
---
Nitpick comments:
In `@App.tsx`:
- Around line 187-189: Move the loadedScopeRef.current assignment out of the App
component render body and synchronize it in a dedicated useEffect keyed to
loadedScope, matching the existing activeGraphIdRef/currentDiagramRef pattern.
Apply the same change to graphsRef.current, using an effect keyed to graphs, and
remove both direct render-time mutations.
🪄 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: db0adcd3-8645-4038-9fdb-663162a215a8
📒 Files selected for processing (17)
.github/workflows/update-supporters.ymlApp.tsxCHANGELOG.mdapi/delete-account.tsapi/generate.tsapi/usage.tsapi/webhooks/polar.tscomponents/LandingPage.tsxpackage.jsonscripts/generate-seo-pages.mjsscripts/seo-content.mjsservices/hostedAi.tsservices/localStore.tsservices/shares.tsservices/useCloudSync.tssupabase/schema.sqlvite.config.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- api/usage.ts
- scripts/seo-content.mjs
- package.json
- CHANGELOG.md
- api/generate.ts
- services/useCloudSync.ts
- services/hostedAi.ts
- scripts/generate-seo-pages.mjs
- api/webhooks/polar.ts
- components/LandingPage.tsx
- services/shares.ts
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
localStorage caps an origin at roughly 5MB, and every account on the browser now shares it. A diagram is ~6KB, but each AI chat turn stores its own full diagram snapshot, so a graph with ten turns is closer to 66KB: the real ceiling was around 75-100 diagrams, not the several hundred a raw count suggests. That same 5MB also holds the Supabase auth token, so filling it could break signing in, not just saving. IndexedDB reports a 10GB quota on the same machine. Diagrams and projects move there; editor preferences stay in localStorage, since they are tiny and shared across accounts by design. The store keeps its shape, so this is contained to localStore.ts plus the call sites that now await. Reads funnel through a single ready() promise, so callers never have to sequence initialisation themselves. Writes are serialised per key so two rapid saves cannot resolve out of order and leave the older array on disk. localStorage remains the fallback when IndexedDB cannot be opened, with a timeout so a database blocked by another tab degrades instead of hanging the app. Migration runs once on first load and is resumable: any namespace already in IndexedDB is left alone, so a partial run can simply be repeated. Both earlier layouts are handled, including a browser that never saw the per-account version. Each key is removed from localStorage as it moves, which is what frees the 5MB. Also asks for persistent storage so the browser does not evict saved diagrams under disk pressure. It is advisory: Chrome grants it on engagement signals and currently declines on localhost. Not compressing. gzip measures 4.5x on real diagram JSON, but IndexedDB stores structured clones rather than UTF-16 strings, so the data is already smaller than it was, and against a 10GB quota the saving buys nothing but CPU on every read. It was only worth considering to stretch the 5MB cap. Verified: 24 scenario tests on the fallback path, and in a real browser the migration moved all three existing namespaces into IndexedDB with identical contents, left zero diagram keys in localStorage, and survived a reload with new writes landing correctly.
Review found that RLS permits an owner to hard-delete a graph or project, and nothing cleaned up after that: - a hard-deleted graph left its public share slug resolving forever, so the diagram stayed readable to anyone holding the link - a hard-deleted project did the same - a hard-deleted graph left its whole version history behind, unreachable but retained until the account was deleted graph_versions now has a real foreign key to graphs with ON DELETE CASCADE (pre-existing orphans are dropped first so the constraint can validate). Shares deliberately keep no foreign key: the payload is a self-contained snapshot, so a diagram can be shared before sync has pushed its row, and a key would reject that insert. The purge trigger handles DELETE explicitly instead, reading OLD, and now fires on delete as well as on the soft-delete flip. Also: - enforce_graph_version_cap takes a transaction-scoped advisory lock keyed on the graph. Two devices inserting at once could each treat the other's row as retained and keep both, so the "cap" could be exceeded. - increment_ai_usage returns -1 for a non-positive limit. The limit was only checked on the conflict path, so the first generation of each month succeeded even with the quota set to zero. Verified against Postgres 16: schema applies cleanly and is idempotent; hard delete now leaves 0 shares and 0 versions (was 1 each); the cap still holds at 100 across 130 inserts; metering returns -1 at limit 0 and counts normally otherwise. Not changed: the review rated "purge_shares_for_deleted_content reads old.deleted on INSERT" as Critical, claiming it breaks every insert once shares exist. It does not. In a PL/pgSQL row-level INSERT trigger OLD is NULL rather than unassigned, so coalesce(old.deleted, false) is fine. Verified directly with shares present across plain insert, insert with deleted = true, project insert, and the tombstone upsert path: all succeed.
Storage writes never reported failure, so a move could destroy the only copy: adoptScope wrote the guest namespace into the account, then cleared the source unconditionally. If the destination write failed (a full quota is the realistic case) the work was gone. lsSet and the IndexedDB helpers now return whether the write actually landed, adoptScope returns null and keeps the source when it did not, and both migrations only drop a source once its copy is on disk. The IndexedDB helper also resolved on request success rather than transaction completion, which reports success for a transaction that later aborts. A failed first cloud pull was treated as proof the account was empty, so signed-out work could be adopted into an account whose cloud actually held diagrams: exactly the merge this design exists to prevent. decideGuestAdoption now takes firstPullFailed and waits instead. The editor no longer blocks on an adoption that can never resolve. Three more from review, all reachable: - scheduleAutosave never cleared its debounce handle, so after the first autosave applyRemote permanently believed edits were in flight and stopped refreshing the open diagram from other devices. - An account switch left historyIndexRef pointing into the old history. Ctrl+Z right after switching passed undo's guard and indexed past the new one-item array, feeding undefined to the canvas. - An account switch left pending history and autosave timers armed, so the outgoing account's diagram could be written into the incoming namespace. Also: a graph deleted on another device stayed open in the editor and kept being re-uploaded. applyRemote now closes it. Verified: 31 store/scenario tests, including a simulated quota failure proving the guest namespace survives a failed adoption, and the four adoption decisions.
api/delete-account: stop trusting our own profile row. A missing profiles row, or a subscription id never written because a webhook was lost, meant deletion proceeded with no billing check and could leave a live subscription charging a deleted account. Ask Polar directly by external customer id, and cancel everything it reports plus anything our row knows about. A failed lookup now returns 503 instead of telling the user to go cancel manually, which also fixes missing Polar configuration being reported as cancel_failed. api/portal: resolve the Polar client outside the try, so an unconfigured deployment answers 503 rather than "No billing account found, wait a few seconds and try again", which sent the user in circles. api/usage: a failed profile lookup was reported as isPro:false with HTTP 200, indistinguishable from a lapsed plan. Now 503, matching /api/generate. services/shares: revokeShare reported success when the delete removed nothing. The delete policy is owner-scoped, so a mismatched id or an RLS denial silently affected zero rows while the UI cleared the link and the URL kept resolving. It now selects the deleted row and errors on an empty result. Cross-account leaks, all the same shape (a response landing after the account changed): the hosted usage meter, the custom template library, and the auth profile, which kept showing the previous account's Supporter status until the replacement query returned. services/keyObfuscation: btoa throws on any character above U+00FF, so a key pasted with a smart quote or non-Latin text crashed the settings save. Now round-trips through UTF-8 bytes. Existing stored keys are ASCII and decode unchanged. vite.config (dev server only): reject path traversal out of api/, and cap the request body at 2MB so one oversized request can't exhaust the dev process. ComponentLibrary: Enter in the template name field called the save handler directly, bypassing the button's disabled state, so repeated presses could create duplicate templates or save a blank name. Verified: key obfuscation round-trips smart quotes, CJK and emoji (all previously threw) and still passes legacy plain values through; every raw traversal path returns 404 with no file contents while /api/usage and the SPA still serve.
Every labelled point on the generated diagram SVGs is supposed to sit on the crossing it names, and a dashed dropline is rendered from it to the axis, so a misplaced point is visible. - monopoly: MR was drawn with demand's slope. For D = AR = 100 - Q the marginal revenue curve is 100 - 2Q (same intercept, twice the slope). MC = MR then lands at Q = 31.9, and P_m reads off demand at 68.1. - negative externalities: MSC was not parallel to MPC, contradicting the page's own "keep MSC parallel to MPC" instruction. Made it a constant external cost of 20; Q* moves to (40, 60). - positive externalities: same problem between MSB and MPB. Both are now slope -1 with an external benefit of 20, putting Q_1 at (45, 45) and Q* at (55, 55). - AD-AS: short-run equilibrium was 4 units off the AD/SRAS crossing. - subsidy: S-sub was not parallel to S, so the vertical gap was not a constant per-unit subsidy. Both equilibria were also off. - perfect competition: Q* sat 1.1 units past where the rising branch of MC cuts the price line.
…py fixes Server: - getAppUrl built the Polar checkout success/cancel URLs straight from the request's Origin (or Host) header. On any deployment that is not Vercel + APP_URL, a caller could point that post-payment redirect at a site of their choosing. Candidates now have to clear an allowlist: APP_URL, the new optional ALLOWED_ORIGINS, and - outside production only - localhost and the dev-tunnel providers already listed in vite.config.ts. - /api/generate had no bound on the upstream model call, so a hung request was only stopped by the platform function timeout, which kills the process before the refund path can run and costs the user a credit for a generation they never got. Added a 30s AbortSignal, a distinct "took too long" message, and an explicit maxDuration so the abort always fires first. - resolveAiClient() rebuilt the client (and re-parsed the service-account JSON) on every request; memoised, since it only reads env vars. - Dropped AiConfig.mode, which was set in all three branches and never read. Content: - Legal page promised "unlimited generations" on a bring-your-own key; now says BYOK is not metered by this app but is subject to the provider's limits and costs. - "Full IB Curriculum" card had dropped development economics. - Shared-view footer rendered "IB EconGraph AI , the free...". - Fixed a sentence with no main clause in the AI system prompt. Docs: - BACKEND_SETUP documented a 3-day billing grace period; the webhook grants 1 (ACTIVE_MARGIN_DAYS). - Labelled the three dotenv code fences (MD040). - dev:api now runs npx vercel dev, so it works without a global CLI install, and the docs say why the CLI is not a devDependency. - CHANGELOG version links pointed at release tags; the repo has no tags or releases at all, so both 1.1.0 and 1.0.0 would 404. Removed them with a note to restore once tagged. - Keepalive curl had no timeout, so a hung connection would hold the runner until GitHub's 6h limit.
…ate rows - AuthModal's email and password inputs had only placeholders, so a screen reader announced no field name. Added aria-labels (three inputs, including the reset form's). - ComparePage: row labels were plain <td>, so a cell could not be associated with its row; the horizontally scrolling wrapper around a min-w-[760px] table had no way to be scrolled by keyboard. Added scope="row"/"col", a named focusable region, and a screen-reader-only name for the empty corner header. - Custom template rows were a div with onClick only, so keyboard users could not add a saved template. They cannot become <button> (they contain the delete button), so they get role/tabIndex/Enter+Space. The delete button was also unnamed and stayed invisible under keyboard focus. - text-gray-400 body text on white is 2.5:1, below WCAG AA's 4.5:1 for normal-size text; moved the legal-page date/footer, the shared-view footer and the comparison-table subtitles to gray-500 (4.8:1).
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/LandingPage.tsx (1)
225-236: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse crawlable links for public Pricing and Compare routes.
These public SEO pages are rendered as buttons, while Privacy and Terms use
RouteLink. UseRouteLinkwith/pricingand/compareso crawlers, keyboard users, and new-tab navigation retain normal link behavior.
components/LandingPage.tsx#L225-L236: replace the top-navigation buttons withRouteLinkinstances.components/LandingPage.tsx#L764-L775: replace the footer buttons with the sameRouteLinkinstances.🤖 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 `@components/LandingPage.tsx` around lines 225 - 236, Replace the Pricing and Compare buttons in the top navigation with RouteLink instances targeting /pricing and /compare, preserving their styling and labels; apply the same replacement to the footer Pricing and Compare controls in components/LandingPage.tsx at lines 764-775. Remove the onOpenPricing and onOpenCompare handlers from these public navigation elements.api/usage.ts (2)
43-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCapture the usage month once per request.
The month used in the database query can roll over before the response is built, causing a previous month’s
usedvalue to be labeled with the new month. Storeconst month = currentUsageMonth()once and reuse it for both the query and response.Proposed fix
+ const month = currentUsageMonth(); const [profile, usageResult] = await Promise.all([ ... - .eq('month', currentUsageMonth()) + .eq('month', month) ... - month: currentUsageMonth(), + month,Also applies to: 57-62
🤖 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 `@api/usage.ts` around lines 43 - 44, Capture the result of currentUsageMonth() once at the start of the request, then reuse that month variable in the database query and response construction. Update the usage handling around the query and the response fields near lines 57–62 so the queried month and returned month cannot diverge.
39-45: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWrap delayed Supabase admin initialization with 503 handling.
getSupabaseAdmin()throws synchronously whenSUPABASE_URL/SUPABASE_SECRET_KEYare missing, and inapi/usage.tsthat happens before the.catchon the auth path plus the 503 handling forusageResult.error. Move this initialization inside atryand return the existing “Usage service is temporarily unavailable” response on failure.🤖 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 `@api/usage.ts` around lines 39 - 45, Update the usage-loading flow around getSupabaseAdmin() so its synchronous initialization occurs inside a try/catch, returning the existing “Usage service is temporarily unavailable” 503 response when initialization or the usage query fails. Preserve the current auth-path handling and successful usage response behavior.
🧹 Nitpick comments (2)
package.json (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Vercel CLI used by
dev:api.
npx vercelcurrently resolves an unpinned CLI version at invocation time because there is no localverceldependency. Add a validatedverceldev dependency and invokevercel devdirectly frompackage.jsonfor reproducible local API behavior.🤖 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 `@package.json` at line 39, Add a validated version of the vercel package to devDependencies, then update the dev:api script to invoke the locally installed vercel CLI directly instead of using npx. Preserve the existing dev command and --listen 4000 option.services/localStore.ts (1)
114-128: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClose a connection that arrives after the fallback.
When
onblockedor the timeout wins,req.onsuccesscan still fire later and hand back a liveIDBDatabasenobody holds. An open connection is exactly what blocks another tab's upgrade, so the fallback can end up perpetuating the condition it's reacting to.♻️ Close the orphaned connection
- req.onsuccess = () => done(req.result); + req.onsuccess = () => { + if (settled) { req.result.close(); return; } + done(req.result); + };🤖 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 `@services/localStore.ts` around lines 114 - 128, Update the IndexedDB request handling around the timeout and onblocked fallback so completion is coordinated and late req.onsuccess results are closed when done has already resolved. Ensure any database returned after the fallback is closed immediately, while preserving the existing successful connection behavior when it wins.
🤖 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 `@api/webhooks/polar.ts`:
- Around line 97-172: Guard the Date.parse results in decideEntitlement for both
current?.pro_until and current?.polar_event_at, treating non-finite or invalid
parses as absent values. Ensure malformed pro_until cannot flow into Math.max or
new Date(...).toISOString(), and malformed polar_event_at does not disable
event-order validation or cause an exception.
In `@services/localStore.ts`:
- Around line 299-306: Update readCollection to inspect the IdB result’s success
status before returning data: preserve the array result only when result.ok is
true, and propagate the underlying read failure otherwise instead of returning
an empty array. Ensure readScope and its callers, including autosave,
scopeHasContent, and adoptScope, retain or surface that failure so failed reads
cannot be treated as empty scopes or trigger destructive writes.
- Around line 226-247: Update the migration flow around the loop in
migrateToIndexedDb to track whether every namespace was processed successfully,
marking the pass unsuccessful when idbGet fails or idbPut fails and leaves
localStorage intact. Call writeVersion(VERSION_INDEXEDDB) only after a clean
pass; preserve retryable source data and avoid recording completion when any
namespace was skipped.
In `@types.ts`:
- Around line 87-93: Update the Graph version fingerprint logic in
services/sync.ts to include the titleSetByUser field alongside diagramData,
title, and caption. Ensure changes to this behavior-affecting flag produce a
distinct cloud snapshot even when the other fingerprinted fields are unchanged.
---
Outside diff comments:
In `@api/usage.ts`:
- Around line 43-44: Capture the result of currentUsageMonth() once at the start
of the request, then reuse that month variable in the database query and
response construction. Update the usage handling around the query and the
response fields near lines 57–62 so the queried month and returned month cannot
diverge.
- Around line 39-45: Update the usage-loading flow around getSupabaseAdmin() so
its synchronous initialization occurs inside a try/catch, returning the existing
“Usage service is temporarily unavailable” 503 response when initialization or
the usage query fails. Preserve the current auth-path handling and successful
usage response behavior.
In `@components/LandingPage.tsx`:
- Around line 225-236: Replace the Pricing and Compare buttons in the top
navigation with RouteLink instances targeting /pricing and /compare, preserving
their styling and labels; apply the same replacement to the footer Pricing and
Compare controls in components/LandingPage.tsx at lines 764-775. Remove the
onOpenPricing and onOpenCompare handlers from these public navigation elements.
---
Nitpick comments:
In `@package.json`:
- Line 39: Add a validated version of the vercel package to devDependencies,
then update the dev:api script to invoke the locally installed vercel CLI
directly instead of using npx. Preserve the existing dev command and --listen
4000 option.
In `@services/localStore.ts`:
- Around line 114-128: Update the IndexedDB request handling around the timeout
and onblocked fallback so completion is coordinated and late req.onsuccess
results are closed when done has already resolved. Ensure any database returned
after the fallback is closed immediately, while preserving the existing
successful connection behavior when it wins.
🪄 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: fb36f763-848c-43e5-a900-f04e9e90a866
📒 Files selected for processing (43)
.github/workflows/db-keepalive.ymlApp.tsxCHANGELOG.mdapi/_lib/polar.tsapi/checkout.tsapi/delete-account.tsapi/generate.tsapi/portal.tsapi/usage.tsapi/webhooks/polar.tscomponents/AccountSection.tsxcomponents/AuthModal.tsxcomponents/CloudHistoryModal.tsxcomponents/ComparePage.tsxcomponents/ComponentLibrary.tsxcomponents/LandingPage.tsxcomponents/LegalPages.tsxcomponents/PricingPage.tsxcomponents/ShareModal.tsxcomponents/SharedViewPage.tsxdocs/BACKEND_SETUP.mdindex.htmlpackage.jsonscripts/generate-seo-pages.mjsscripts/seo-content.mjsscripts/update-supporters.mjsservices/aiProvider.tsservices/auth.tsxservices/billing.tsservices/diagramPrompt.tsservices/entitlement.tsservices/hostedAi.tsservices/httpTimeout.tsservices/keyObfuscation.tsservices/localStore.tsservices/openrouter.tsservices/shares.tsservices/sync.tsservices/useCloudSync.tssupabase/schema.sqltypes.tsvercel.jsonvite.config.ts
🚧 Files skipped from review as they are similar to previous changes (31)
- services/keyObfuscation.ts
- api/checkout.ts
- .github/workflows/db-keepalive.yml
- scripts/seo-content.mjs
- components/LegalPages.tsx
- services/aiProvider.ts
- scripts/update-supporters.mjs
- services/billing.ts
- components/PricingPage.tsx
- services/hostedAi.ts
- api/portal.ts
- CHANGELOG.md
- api/_lib/polar.ts
- components/AuthModal.tsx
- components/ComponentLibrary.tsx
- components/CloudHistoryModal.tsx
- api/generate.ts
- components/SharedViewPage.tsx
- components/ComparePage.tsx
- components/ShareModal.tsx
- vite.config.ts
- services/openrouter.ts
- services/useCloudSync.ts
- docs/BACKEND_SETUP.md
- services/shares.ts
- supabase/schema.sql
- scripts/generate-seo-pages.mjs
- services/sync.ts
- services/auth.tsx
- components/AccountSection.tsx
- App.tsx
Comment on lines
+87
to
+93
| /** | ||
| * Set once the user names the graph themselves (rename dialog, or editing the | ||
| * title on the canvas). While it is unset the AI is free to retitle the graph | ||
| * on each generation. Absent on graphs saved before this flag existed, which | ||
| * fall back to a title-based heuristic. | ||
| */ | ||
| titleSetByUser?: boolean; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\bversionFingerprint\b|\btitleSetByUser\b' --glob '*.ts' --glob '*.tsx' .Repository: Sukarth/IB-EconGraph-AI
Length of output: 3583
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== services/sync.ts relevant sections =="
sed -n '1,130p' services/sync.ts
sed -n '480,535p' services/sync.ts
echo
echo "== app graph mutation/rename sections =="
sed -n '400,440p' App.tsx
sed -n '600,630p' App.tsx
sed -n '825,842p' App.tsx
echo
echo "== contentHash definition/usages =="
rg -n -C2 'function contentHash|const contentHash|contentHash\(' --glob '*.ts' --glob '*.tsx' .
echo
echo "== structural facts about Graph shape and versionFingerprint inputs =="
python3 - <<'PY'
from pathlib import Path
sync = Path('services/sync.ts').read_text()
types = Path('types.ts').read_text()
app = Path('App.tsx').read_text()
print('versionFingerprint references titleSetByUser:', 'titleSetByUser' in sync[sync.index('function versionFingerprint'):sync.index('function versionFingerprint')+1000])
print('versionFingerprint JSON stringifies:', 'JSON.stringify({' in sync)
start = sync.index('function versionFingerprint')
end = sync.index('}', start) + 1
print(sync[start:end])
PYRepository: Sukarth/IB-EconGraph-AI
Length of output: 12727
Include titleSetByUser in the version fingerprint.
Graph can change this behavior-affecting flag without changing diagramData, title, or caption for existing graphs, but services/sync.ts now only fingerprints those three fields. This allows cloud versioning to skip a snapshot for the same content plus a renamed/user-set graph flag.
🤖 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 `@types.ts` around lines 87 - 93, Update the Graph version fingerprint logic in
services/sync.ts to include the titleSetByUser field alongside diagramData,
title, and caption. Ensure changes to this behavior-affecting flag produce a
distinct cloud snapshot even when the other fingerprinted fields are unchanged.
The reconciliation query selected `data` for every graph the user owns, so each sync downloaded the entire library just to compare timestamps, and then threw almost all of it away. An 80-diagram library cost ~3.2 MB per sync even when nothing had changed. Supabase's free tier meters egress bytes, not request count, so splitting this into two round trips is a straight win: phase one selects only the metadata columns needed to reconcile, phase two fetches `data` for the ids that reconciliation decided to pull. A no-change sync now transfers a few KB and issues no second request at all. Phase two chunks its id list at 100 because `in.(...)` filters travel in the query string. Rows that disappear between the two phases are skipped rather than treated as empty. Projects stay single-phase: their payload *is* their metadata, so there would be nothing left to defer.
readCollection discarded the `ok` flag that IdbResult exists to carry, so a failed IndexedDB read came back as []. Three things then acted on that emptiness as if it were fact: the autosave effects wrote the empty arrays over the stored records, scopeHasContent reported nothing worth keeping, and adoptScope copied nothing into the destination and cleared the source anyway, which is precisely the loss its doc comment claims to prevent. readScope now returns `ok`. adoptScope refuses to move a namespace it could not read, scopeHasContent answers false rather than guessing, and App leaves `loadedScope` unset so the save effects stay parked for the session. A banner says saving is off rather than letting the session look normal while nothing persists. Separately, migrateToIndexedDb stamped VERSION_INDEXEDDB even when a namespace had been skipped. Both skip paths deliberately leave the localStorage source in place to be retried, but the stamp ended the retries while reads had already moved to IndexedDB, so one transient failure orphaned that namespace permanently. Only stamp a clean pass. Also guard the two Date.parse calls in decideEntitlement. Its CurrentBillingState is a plain interface, not a row type, so nothing guarantees the strings parse; a NaN reaching Math.max made new Date(...).toISOString() throw, and a webhook that throws is one Polar retries forever.
There was a problem hiding this comment.
10 issues found across 43 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="api/checkout.ts">
<violation number="1" location="api/checkout.ts:61">
P1: A second checkout can still be created before the first payment webhook updates `profiles`, so separate tabs/devices can create parallel subscriptions and double-charge. The client-side disabled button is not a cross-request lock; retain an authoritative Polar check or add a server-side pending-checkout lock before creating a session.</violation>
</file>
<file name="services/diagramPrompt.ts">
<violation number="1" location="services/diagramPrompt.ts:160">
P2: BYOK Gemini and OpenRouter generations still bypass this validator and can pass partial model output into the renderer. Apply `diagramShapeError` after parsing in both provider implementations before returning `DiagramData`.</violation>
<violation number="2" location="services/diagramPrompt.ts:213">
P2: A malformed annotation without `label` passes validation, then `FormattedText` calls `text.length` on `undefined` while rendering it. Validate the required annotation fields, not only its coordinates.</violation>
</file>
<file name="api/_lib/polar.ts">
<violation number="1" location="api/_lib/polar.ts:28">
P3: Checkout testing through `*.ngrok.app` will fail after Polar redirects because this allowlist accepts the origin while Vite rejects its Host header. Add `.ngrok.app` to `server.allowedHosts` or remove it here so the two lists stay aligned.</violation>
</file>
<file name="vercel.json">
<violation number="1" location="vercel.json:5">
P2: The `functions` block in `vercel.json` configures a serverless function timeout, which violates the project convention (README.md) that `vercel.json` is for routing/rewrites only. Export a `config` object from `api/generate.ts` instead, which is supported by `@vercel/node` and co-locates the timeout with the handler.</violation>
</file>
<file name="api/delete-account.ts">
<violation number="1" location="api/delete-account.ts:54">
P2: Account deletion now fails for every non-billing deployment, including users with no subscription, because billing is queried unconditionally. Skip the Polar lookup when billing is unconfigured and the profile has no `polar_subscription_id`; retain the fail-closed path when an existing billing ID might still need revocation.</violation>
</file>
<file name="components/ComponentLibrary.tsx">
<violation number="1" location="components/ComponentLibrary.tsx:260">
P2: Screen readers may not expose the Delete control independently because it is nested inside an element with `role="button"`. Keep the row activation control separate from the delete button (for example, make only a sibling/inner non-overlapping element the keyboard-activatable control).</violation>
</file>
<file name="services/sync.ts">
<violation number="1" location="services/sync.ts:591">
P2: A revision still disappears if the graph changes before its queued retry. Persist each failed version row/snapshot, not only its ID, so retrying inserts the version whose initial write failed.</violation>
</file>
<file name="supabase/schema.sql">
<violation number="1" location="supabase/schema.sql:51">
P2: Existing subscribers keep `polar_event_at = NULL`, so their first delayed pre-migration `subscription.active` bypasses ordering and can restore access after a cancellation. Seed/reconcile the last applied event for existing billing rows before enabling this comparison.</violation>
<violation number="2" location="supabase/schema.sql:302">
P2: Concurrent multi-graph history syncs can deadlock instead of serializing: opposite graph orders hold transaction-scoped advisory locks cyclically. Acquire batch locks in a stable graph-id order (for example in a statement-level trigger) before pruning.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return res.status(503).json({ error: 'Billing is not configured on this deployment.' }); | ||
| } | ||
|
|
||
| // Deliberately NOT calling polar.subscriptions.list() here to close the gap |
There was a problem hiding this comment.
P1: A second checkout can still be created before the first payment webhook updates profiles, so separate tabs/devices can create parallel subscriptions and double-charge. The client-side disabled button is not a cross-request lock; retain an authoritative Polar check or add a server-side pending-checkout lock before creating a session.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/checkout.ts, line 61:
<comment>A second checkout can still be created before the first payment webhook updates `profiles`, so separate tabs/devices can create parallel subscriptions and double-charge. The client-side disabled button is not a cross-request lock; retain an authoritative Polar check or add a server-side pending-checkout lock before creating a session.</comment>
<file context>
@@ -44,15 +43,32 @@ export default async function handler(req: VercelRequest, res: VercelResponse) {
+ return res.status(503).json({ error: 'Billing is not configured on this deployment.' });
+ }
+
+ // Deliberately NOT calling polar.subscriptions.list() here to close the gap
+ // between a payment succeeding and its webhook landing. That check costs a
+ // Polar API call on every checkout attempt, on an endpoint any signed-in
</file context>
| -- see the other's row as still-retained and both keep it, leaving more than | ||
| -- the cap. The lock is transaction-scoped and keyed on the graph, so it only | ||
| -- ever blocks a concurrent insert for that same graph. | ||
| perform pg_advisory_xact_lock(hashtextextended(new.graph_id::text, 0)); |
There was a problem hiding this comment.
P2: Concurrent multi-graph history syncs can deadlock instead of serializing: opposite graph orders hold transaction-scoped advisory locks cyclically. Acquire batch locks in a stable graph-id order (for example in a statement-level trigger) before pruning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/schema.sql, line 302:
<comment>Concurrent multi-graph history syncs can deadlock instead of serializing: opposite graph orders hold transaction-scoped advisory locks cyclically. Acquire batch locks in a stable graph-id order (for example in a statement-level trigger) before pruning.</comment>
<file context>
@@ -265,6 +295,12 @@ security definer
+ -- see the other's row as still-retained and both keep it, leaving more than
+ -- the cap. The lock is transaction-scoped and keyed on the graph, so it only
+ -- ever blocks a concurrent insert for that same graph.
+ perform pg_advisory_xact_lock(hashtextextended(new.graph_id::text, 0));
+
delete from public.graph_versions
</file context>
|
|
||
| -- Added after the initial release; `create table if not exists` above skips | ||
| -- existing installs, so bring them forward explicitly. | ||
| alter table public.profiles add column if not exists polar_event_at timestamptz; |
There was a problem hiding this comment.
P2: Existing subscribers keep polar_event_at = NULL, so their first delayed pre-migration subscription.active bypasses ordering and can restore access after a cancellation. Seed/reconcile the last applied event for existing billing rows before enabling this comparison.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At supabase/schema.sql, line 51:
<comment>Existing subscribers keep `polar_event_at = NULL`, so their first delayed pre-migration `subscription.active` bypasses ordering and can restore access after a cancellation. Seed/reconcile the last applied event for existing billing rows before enabling this comparison.</comment>
<file context>
@@ -37,10 +37,19 @@ create table if not exists public.profiles (
+-- Added after the initial release; `create table if not exists` above skips
+-- existing installs, so bring them forward explicitly.
+alter table public.profiles add column if not exists polar_event_at timestamptz;
+
alter table public.profiles enable row level security;
</file context>
| * `isAllowedOrigin`), where they exist so Polar redirects and webhooks can be | ||
| * tested against a real HTTPS origin. | ||
| */ | ||
| const DEV_TUNNEL_SUFFIXES = ['.devtunnels.ms', '.ngrok-free.app', '.ngrok.app', '.trycloudflare.com']; |
There was a problem hiding this comment.
P3: Checkout testing through *.ngrok.app will fail after Polar redirects because this allowlist accepts the origin while Vite rejects its Host header. Add .ngrok.app to server.allowedHosts or remove it here so the two lists stay aligned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At api/_lib/polar.ts, line 28:
<comment>Checkout testing through `*.ngrok.app` will fail after Polar redirects because this allowlist accepts the origin while Vite rejects its Host header. Add `.ngrok.app` to `server.allowedHosts` or remove it here so the two lists stay aligned.</comment>
<file context>
@@ -15,29 +15,76 @@ export function getPolar(): Polar {
+ * `isAllowedOrigin`), where they exist so Polar redirects and webhooks can be
+ * tested against a real HTTPS origin.
+ */
+const DEV_TUNNEL_SUFFIXES = ['.devtunnels.ms', '.ngrok-free.app', '.ngrok.app', '.trycloudflare.com'];
+
+/** Origins this deployment is willing to redirect a checkout back to. */
</file context>
Last commit left `loadedScope` unset after a failed read so the save effects would skip. That was wrong in both directions: the effect's own guard is `storeScope === loadedScope`, so it would re-run forever, and `loadedScope` kept pointing at the *previous* account, which meant switching back to it wrote this namespace's empty arrays over its real library. `loadedScope` now records the namespace this effect settled, whichever way it went, and `storageUnreadable` decides separately whether saving is allowed. Switching back to a readable account clears it and resumes. lsGet had the same conflation the IndexedDB path did: a localStorage that throws returned null, indistinguishable from a missing key, so the fallback backend reported a readable empty scope. It now reports `ok`. migrateToNamespaces also stamped its version unconditionally. A collection that failed to copy stays in the guest keys, so ending the retries there reclassifies an account's diagrams as work done signed out and they vanish from the account they belong to. It now leaves the version and the owner key alone until the whole move lands.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
fetchWithTimeout passed `signal: controller.signal` after spreading `init`, so a caller's own AbortSignal was silently dropped. Nothing passes one today, which is exactly why it would have been found the hard way. The two signals are now chained, and a caller's abort propagates as their own error rather than being relabelled a timeout. graph_versions rows go in as one multi-row insert, so one transaction, and the per-row cap trigger takes a transaction-scoped advisory lock per graph as the rows land. Two devices pushing an overlapping set in their own library order take those locks in opposite orders, which is a deadlock rather than the serialisation the lock was added for. Sorting by graph id gives every client one order. FormattedText called text.length unguarded. Labels are required by the generation schema but OpenRouter has no schema, and diagrams can be hand-edited or predate a field, so a missing label took down the whole canvas. It renders as nothing now. Deliberately not added to diagramShapeError: a missing label is cosmetic, and throwing away an otherwise good generation over it is the worse failure.
The delete button stops click propagation, but keydown still bubbles to the row's Enter/Space handler, so deleting a template from the keyboard also added it to the canvas. The row now ignores keys that did not originate on the row itself. Also record why polar_event_at is left NULL on existing rows rather than seeded. Seeding now() would reject every event stamped before the migration, including a slow legitimate renewal, and cutting off someone who paid is worse than one unordered event. There is no value that reconstructs the real last-applied time, so the choice is between two imperfect options and this is the recoverable one.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The pending-version queue, the share-refresh marker and the version hash map were all stored under one key for the whole browser. With two accounts on one machine, B's sync read A's queued graph ids, found those graphs missing from its own library, concluded they were deleted and dropped them; B's successful share refresh cleared the flag A was still waiting on. Each is now keyed by user. Existing queues are abandoned once, which costs at most a duplicate snapshot. Guest adoption cleared `pendingGuestAdoption` before awaiting adoptScope, so for the length of that copy the auto-open effect saw an empty library, created a blank diagram and synced it up. The adopted graphs then replaced it locally while the stray row stayed in the cloud. A separate in-flight flag now covers the await, cleared in a finally so a cancelled run cannot strand the editor empty. handleImportData awaited fetchCloudIds with no scope binding, so signing in or out mid-restore dropped the backup into the other account. It is now bound to the namespace it started in and abandons cleanly if that changed, with every tombstone write deferred past the check so an abandoned restore leaves no trace in either account. Two follow-ups on the previous commit, both correctly flagged: the timeout wrapper classified from the signals' final state, so a caller cancelling just after the deadline was reported as their abort rather than a timeout, and it dropped the caller's abort reason. It now records which fired first and forwards the reason. migrateToNamespaces ignored whether clearing the guest key succeeded, so a failed clear left the same diagrams in both namespaces and still stamped the version. A resumed run could not repair it either, because the destination was populated by then and the don't-clobber guard skipped it. It now finishes the move when the destination already holds exactly the content being migrated.
Constraint names are unique per table, not per database, so the idempotency guard around graph_versions_graph_id_fkey could match a same-named constraint on some other table and skip adding the foreign key it exists to add. Verified against Postgres 16 with a decoy constraint: unscoped, the FK is never created; scoped by conrelid, it is. api/_lib/polar.ts accepts .ngrok.app as a checkout redirect origin but vite's allowedHosts did not, so that tunnel fails with "This host is not allowed" only after Polar redirects back.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
`pendingGuestAdoption` is in this effect's dependency list, so setting it false to settle the decision re-runs the effect, and the cleanup trips `cancelled` long before adoptScope resolves. By then adoptScope has copied the diagrams into the account and emptied the guest namespace, so discarding its result left the work written to disk but missing from state, and the auto-open effect then created a blank graph and autosaved it over the top. Signing in over guest work could destroy it. The cancel flag conflated "this effect re-ran" with "the account changed". Only the second is a reason to withhold, and even then the diagrams are safe on disk in the namespace they were moved into, so the check is now against the live scope. Predates the previous commit; the in-flight flag added there held the auto-open effect back but did not stop the transfer being discarded. Verified by reading rather than execution: exercising this path needs an account with no diagrams, and creating one is not something I can do.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…g refs The previous commit decided whether to publish an adopted library by comparing loadedScopeRef against the scope the copy started in. That ref is synced by a passive effect, so there is a window where the incoming account's data has already been loaded into state but the ref still names the outgoing one. A copy resolving in that window passed the check and its graphs were published under, and saved into, the wrong account. The scope-load effect now invalidates the handover directly, on the line before its first await. That runs synchronously when the switch is detected, so the new account's data cannot be live while an old handover still looks current. It only runs on a real scope change, since the effect returns early when the loaded scope already matches.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Third variant of the same failure. The token was cleared when a switch started, but switching away and straight back leaves the scope-load effect early-returning, because the namespace never stopped being the loaded one. Nothing restored the token, so the handover was dropped, and auto-open wrote a blank graph over diagrams adoptScope had already moved to disk. Every version of this has tried to decide, from inside an async callback, whether the namespace was still live, using state captured before the await. That question cannot be answered reliably there, and answering it wrong destroys the user's work, because by then the copy has emptied the guest namespace and this is the only copy in memory. So stop deciding. The callback records the result and an effect publishes it when the live scope matches, reading state as it actually is. An account switch mid-copy parks the diagrams until that account is back; a switch that completes clears them, because the load effect's own read of the disk already includes them. Auto-open waits on a parked handover only for the namespace it belongs to, so another account still gets its first diagram.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="App.tsx">
<violation number="1" location="App.tsx:323">
P1: A rapid account switch can still discard an adopted guest library and replace it with a blank graph. Preserve a parked handover when loading a different account; only clear one whose scope is the namespace being loaded.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // whatever the read (or a sync that ran meanwhile) turned up. Note this sits | ||
| // after the early return above: coming back to a namespace that never | ||
| // stopped being the loaded one does no read, and must not discard anything. | ||
| setPendingAdopted(null); |
There was a problem hiding this comment.
P1: A rapid account switch can still discard an adopted guest library and replace it with a blank graph. Preserve a parked handover when loading a different account; only clear one whose scope is the namespace being loaded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At App.tsx, line 323:
<comment>A rapid account switch can still discard an adopted guest library and replace it with a blank graph. Preserve a parked handover when loading a different account; only clear one whose scope is the namespace being loaded.</comment>
<file context>
@@ -312,12 +314,13 @@ export default function App() {
+ // whatever the read (or a sync that ran meanwhile) turned up. Note this sits
+ // after the early return above: coming back to a namespace that never
+ // stopped being the loaded one does no read, and must not discard anything.
+ setPendingAdopted(null);
let cancelled = false;
void (async () => {
</file context>
Suggested change
| setPendingAdopted(null); | |
| setPendingAdopted((adopted) => adopted?.scope === storeScope ? null : adopted); |
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
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.
Adds an optional account and Supporter tier on top of the existing editor, plus build-time SEO landing pages. The core editor stays free and fully offline: with none of the new environment variables set, every cloud feature hides itself and the app behaves exactly as it does today.
Squashed to a single commit.
mainwas still 1.0.0 and had none of this; 1.1.0 was never tagged or released, so everything here ships as 1.1.0.What this adds
Accounts and cloud (Supabase)
get_share()RPC so thesharestable itself is never readable byanon(no bulk enumeration).supabase/schema.sqlwith row-level security on every table. Entitlement is a single rule,pro_until > now(), enforced identically in SQL (is_pro()) and TypeScript (services/entitlement.ts).Hosted AI
/api/generateruns generation server-side for supporters, so they need no API key.docs/BACKEND_SETUP.mdsection 2.Billing (Polar)
SEO
sitemap.xml, generated at build time intodist/. Self-contained HTML, no external CSS or JS.public/sitemap.xmlis removed because it is now generated.Ops
db-keepalive.ymlpings the database every ~5 days so a free-tier Supabase project never pauses after 7 days idle.update-supporters.ymlrefreshes the supporters list weekly.Licensing
Testing
Verified against a live Supabase project, Polar sandbox, and Vertex AI using a real service-account key.
not_proClient UI checked page by page: landing, pricing, compare, privacy, terms, editor (drawing, templates, export), dashboard, settings and the auth modal.
Not covered here: the production Vercel runtime and production Polar, Google OAuth, password reset, quota exhaustion at 150, and subscription lifecycle past activation (cancel, past-due, renewal). These need a smoke test on the live deployment after deploy.
Deploying
Set the environment variables in
docs/BACKEND_SETUP.mdsection 4, point a production Polar webhook at/api/webhooks/polar, add theSUPABASE_URLandSUPABASE_SECRET_KEYrepository secrets so the scheduled workflows run, and applysupabase/schema.sql.Summary by Sourcery
Add optional cloud-backed Supporter plan with accounts, hosted AI, billing, and SEO landing pages while keeping the core editor fully free and offline-compatible.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
Summary