fix(premium): fan out ALL PRO-gated loaders on Free→Pro + close supply-chain race#3828
Conversation
…y-chain race
Two empty-on-mobile bug reports, one root cause: PRO-gated loaders silently
strand when hasPremiumAccess() returns false at the boot tick.
## Symptom 1 — PREMIUM STOCK ANALYSIS empty body
data-loader.ts:456 gates loadStockAnalysis on `hasPremiumAccess() && shouldLoad(...)`.
If Clerk/Convex haven't hydrated by the loadAllData(true) boot call, the task
is skipped. App.ts:1017-1031 firePremiumLoaders only re-fired loadTradePolicy
on Free→Pro — every other PRO-gated loader was on its own. The scheduled
refresh that WOULD have caught up is gated to SITE_VARIANT === 'finance'
(App.ts:1495), so on the full variant the panel sits empty for the entire
session. Secondary contributor: Panel.ts:954 showRetrying and Panel.ts:1045
setContent both `if (this._locked) return;` silently, so even if loadStockAnalysis
DID run while the panel was rendering its locked CTA, the data render was
swallowed; unlockPanel then restored the pre-lock _savedContent (an empty body).
Fix: expand firePremiumLoaders to fan out stock-analysis, stock-backtest,
daily-market-brief, market-implications, wsb-tickers, and resilience-ranking.
Each loader is idempotent and self-checks hasPremiumAccess() internally, so
the calls are safe to fire even if they were already covered by a scheduler.
## Symptom 2 — Route Explorer "No modeled lane" for HK→DE (Pro user)
src/services/supply-chain/index.ts had 8 fetchers gated by the BARE
`hasPremiumAccess()` (no auth state) which reads only the cached isProUser()
Convex tier signal. RouteExplorer.fetchLane uses `hasPremiumAccess(getAuthState())`
which ALSO accepts Clerk's user.role === 'pro'. The two diverge during the
Clerk-resolved-but-Convex-stale window: RouteExplorer's outer gate passes, then
fetchRouteExplorerLane's inner gate fails and returns `{ ...emptyRouteExplorerLane,
...args }` — which has noModeledLane: true. The UI then renders "No modeled
lane for this pair", the SAME message shown when the dataset genuinely lacks
the lane. The user can't tell the two cases apart.
Fix: introduce a `gatedPremium()` helper that always calls
`hasPremiumAccess(getAuthState())`, and route all 8 supply-chain fetcher
gates through it. Eliminates the divergence window for every supply-chain
consumer in one shot.
## Audit-locking regression test
tests/premium-loaders-fan-out-coverage.test.mts statically extracts every
`this.loadX()` call sitting inside an `if (hasPremiumAccess(...))` block in
data-loader.ts and asserts each appears in App.ts:firePremiumLoaders (with
a documented ALLOWLIST escape hatch). The next contributor who adds a
PRO-gated loader without wiring fan-out gets a CI failure with a clear
"add `void this.dataLoader.loadX();` to firePremiumLoaders" message.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes two related empty-panel bugs caused by PRO-gated loaders not being re-fired on the Free→Pro entitlement transition during the Clerk/Convex hydration race. The fix operates on three fronts: expanding
Confidence Score: 4/5Safe to merge — the functional fixes in App.ts and supply-chain/index.ts are correct and well-scoped; the only concern is a coverage gap in the new regression test. The App.ts fan-out expansion and the gatedPremium() helper are straightforward and correct. The regression test has a blind spot: loadWsbTickers at data-loader.ts:569 uses the inline braceless gate form that extractGatedLoaders cannot match, so removing that loader from firePremiumLoaders in the future would not produce a CI failure. tests/premium-loaders-fan-out-coverage.test.mts — the extractGatedLoaders regex needs to handle the inline gate form at data-loader.ts:569 to fully enforce the coverage guarantee it advertises. Important Files Changed
Reviews (1): Last reviewed commit: "fix(premium): fan out ALL PRO-gated load..." | Re-trigger Greptile |
| // boot if Pro hasn't hydrated yet: | ||
| // (a) `if (hasPremiumAccess() && shouldLoad('X')) { tasks.push(...load...()) }` | ||
| // (b) `if (hasPremiumAccess()) { await Promise.allSettled([this.load...(), ...]) }` | ||
| // Match both by lazily consuming up to the next top-level `}` (6-space | ||
| // indented close) and pulling every `this.loadX()` call within. | ||
| const re = /if\s*\(\s*hasPremiumAccess\([^)]*\)[^)]*\)\s*\{([\s\S]*?)\n\s{4,6}\}/g; | ||
| for (const match of src.matchAll(re)) { | ||
| const block = match[1] ?? ''; | ||
| const callRe = /this\.load([A-Z][A-Za-z0-9]+)\(\)/g; | ||
| for (const callMatch of block.matchAll(callRe)) { | ||
| loaders.add(`load${callMatch[1]}`); | ||
| } | ||
| } | ||
| return loaders; |
There was a problem hiding this comment.
Regex misses inline (no-brace) gate form —
loadWsbTickers is unguarded
extractGatedLoaders requires a block opening brace (\)\s*\{) to terminate the if-condition match. data-loader.ts:569 uses the inline form with no braces: if (hasPremiumAccess() && shouldLoad('wsb-ticker-scanner')) tasks.push(...). The regex fails for two reasons: (1) the nested ) from shouldLoad(...) causes the [^)]*\) anchor to consume the wrong paren, leaving the outer ) unmatched before \{, and (2) there is no { at all. As a result loadWsbTickers never enters gatedLoaders and the third assertion silently skips it — removing void this.dataLoader.loadWsbTickers() from firePremiumLoaders would not fail CI. Either convert line 569 of data-loader.ts to a block-brace form, or extend the regex to cover the braceless/shouldLoad variant.
…cause, fix test regex Three follow-ups to PR #3828 review findings: ## P1: Reverted supply-chain auth-aware change — it was a no-op Reviewer is correct: bare `hasPremiumAccess()` already includes the Clerk signal because `panel-gating.ts:31` calls `isProUser()`, and `widget-store.ts:175` `isProUser()` already does `getAuthState().user?.role === 'pro'`. So `hasPremiumAccess()` and `hasPremiumAccess(getAuthState())` return identically — passing auth state only saves a redundant inner check. My `gatedPremium()` helper closed no race window. Reverted the 8 supply-chain call sites + helper + comment. ## P1 followup: actual HK→DE root cause — port-cluster data is wrong Hong Kong's port cluster in `scripts/shared/country-port-clusters.json` only lists `["china-us-west", "intra-asia-container"]`. Germany lists the Asia-Europe routes (`china-europe-suez`, `asia-europe-cape`, `transatlantic`). Zero overlap → `sharedRoutes.length === 0` → `noModeledLane = true`. The Route Explorer message is technically correct: the lane wasn't modeled. The DATA was wrong — HK is one of the world's busiest container ports and ships to Europe via Suez routinely. Added `china-europe-suez` and `asia-europe-cape` to HK's cluster. HK→DE now has 2 shared routes (was 0). Verified other Asian ports (TW, KR, JP, VN, TH, PH) have the same gap — left for a follow-up PR since each needs review against actual shipping data. ## P2: Fan-out coverage test was extracting the wrong loaders Reviewer caught that the regex missed shape (c) single-line gates like `if (hasPremiumAccess() && shouldLoad('wsb-ticker-scanner')) tasks.push(...)`. On investigation it was worse: the broadened regex ONLY matched shape (b) `if (hasPremiumAccess()) { ... }`. The other loaders happened to appear inside an init() handler that ALSO matches shape (b), giving false confidence that the production gates at lines 456/459/462 were covered. Replaced the regex-only approach with a line-walking extractor: 1. Find every line containing `hasPremiumAccess(` 2. Filter to `if`/`else if` gates 3. Collect calls from the line itself (covers shape c) 4. If the line ends with `{`, walk forward with brace-depth tracking until the matching close (covers shapes a and b) Now extracts all 6 PRO-gated loaders (was 4, with 2 of those caught coincidentally via init()). Added a fixture test that proves all 3 shapes extract correctly — a future "regex simplification" that drops a shape fails immediately with a clear message.
koala73
left a comment
There was a problem hiding this comment.
Both findings valid — thanks for catching them. Fixed in 727cdb896:
P1 (no-op supply-chain change): confirmed and reverted. You're right — panel-gating.ts:31 calls isProUser(), and widget-store.ts:175 isProUser() already does getAuthState().user?.role === 'pro', so bare hasPremiumAccess() and hasPremiumAccess(getAuthState()) are identical. My helper was pure ceremony. Reverted the 8 call sites + helper + the misleading comment.
Actual HK→DE root cause was a data bug, not an auth race. scripts/shared/country-port-clusters.json:93 lists HK's nearestRouteIds as ["china-us-west", "intra-asia-container"] only — zero overlap with DE's ["china-europe-suez", "asia-europe-cape", "transatlantic"] → sharedRoutes.length === 0 → noModeledLane: true. HK is one of the world's busiest container ports and ships to Europe via Suez routinely; the data was just missing the Asia-Europe routes. Added them. HK→DE now has 2 shared routes (was 0).
Verified the same gap exists in TW/KR/JP/VN/TH/PH clusters — left for a follow-up PR since each pair needs review against real shipping data, not a one-shot copy of CN.
P2 (test regex gap): worse than the review surfaced. The old regex was matching only shape (b) if (hasPremiumAccess()) { … }; the other loaders happened to appear inside an init() handler that also matches shape (b), masking the gap. Replaced regex with a line-walker (find hasPremiumAccess( in an if/else if, collect calls from the line itself for single-line gates, then walk forward with brace-depth tracking for braced gates). Now extracts all 6 PRO-gated loaders (was 4, with 2 coincidental). Added a fixture test that exercises all 3 shapes so a future regex "simplification" that drops one fails immediately.
…+ lock down (#3832) * fix(route-explorer): close Asia→DE lane gap for TW/KR/JP/VN/TH/PH/IN + lock down PR #3828 fixed HK→Germany by adding `china-europe-suez` and `asia-europe-cape` to HK's `nearestRouteIds` in `scripts/shared/country-port-clusters.json`, and explicitly deferred TW/KR/JP/VN/TH/PH because each needed review against actual shipping data. While triaging a reporter's HK→DE / Premium Stock / WM Analyst empty-state screenshots (pr-3718 session), I confirmed those six countries plus IN still fail with `noModeledLane: true` against DE — server reports the gap honestly, UI shows "No modeled lane for this pair." Root cause for each is identical to HK's: the country has only Pacific/intra-Asia/Gulf-oil route IDs, none of which appear in DE's cluster (`china-europe-suez`, `asia-europe-cape`, `transatlantic`). Server intersection at server/worldmonitor/supply-chain/v1/get-route-explorer-lane.ts:233-234 returns zero shared routes → noModeledLane=true. Fix: add `china-europe-suez` and `asia-europe-cape` (the trunk Asia↔Europe container routes terminating at Rotterdam) to TW, KR, JP, VN, TH, PH, IN. Both routes are tagged `category: 'container', status: 'active'` in src/config/trade-routes.ts; they are factually correct for every major Asian export hub shipping to Northern Europe. Lock-down: tests/country-port-clusters-asia-europe-lane.test.mts (3 cases) 1. Data invariant — every Asian port country in {CN,HK,TW,JP,KR,SG,MY,ID, TH,VN,PH,IN} must have a non-empty `nearestRouteIds` intersection with DE in the cluster JSON. Reverting any of the seven new entries flips this test red with a clear "add china-europe-suez and/or asia-europe- cape to <ISO2>'s nearestRouteIds" error. 2. Computed lane — `computeLane('HK','DE','85','container')` must return `noModeledLane: false` and a non-empty `primaryRouteId`. This is the EXACT shape of the reporter's screenshot (HK→Germany, Electrical & Electronics HS2=85, Container, AUTO badge); the original PR #3828 had no test pinning this case, so a future contributor reverting the HK entry would not be caught. 3. Full Asian-port matrix — same `noModeledLane: false` assertion for all 12 Asian export hubs against DE, defending the algorithm boundary (computeLane) on top of the data invariant. Bite-test: reverting just the data change (git stash on the JSON) flips tests #1 and #3 red, naming the exact 7 offenders (TW/JP/KR/TH/VN/PH/IN); test #2 stays green because HK was already fixed by #3828. Confirms each assertion bites its intended regression. The 30-query smoke matrix in tests/route-explorer-lane.test.mts (38 cases) stays 38/38 green. tsc clean. This complements PR #3828's HK fix and clears the deferred-follow-up note without touching unrelated areas. The reporter's third screenshot (HK→DE "No modeled lane") will resolve on next deploy regardless of this PR (PR #3828 already fixed HK); this PR additionally prevents TW/KR/JP/VN/TH/ PH/IN from showing the same dead-end to other Asia-export-hub users, and makes both fixes regression-proof. * fix(route-explorer): address PR #3832 review P1 — IN→DE picks india-europe, not china-europe-suez Reviewer (correct): adding `china-europe-suez` + `asia-europe-cape` to IN satisfied `noModeledLane === false` for IN→DE but pushed the resolver to pick `china-europe-suez` (Shanghai → Rotterdam) as the primary route. The Route Explorer UI highlights the route's from→to ports, so an Indian shipment to Germany rendered a Shanghai origin port. The test was vacuous on this dimension — it only asserted `noModeledLane === false`, never which route resolved. Root cause of my original choice: DE's `nearestRouteIds` didn't list `india-europe`, so no intersection existed for IN→DE via that route. I papered over the missing intersection by polluting IN's profile with China-origin trunk routes instead of fixing DE's profile. Correct fix (follows existing convention): every European country that already has `china-europe-suez` (the Asia-Europe trunk to Rotterdam) also gets `india-europe` (the India-Europe trunk to Rotterdam). Both routes terminate at the same Northern-European hub; the data model treats trunk routes as broadly serving any European port country. Applied uniformly to DE, GB, FR, IT, ES, NL, BE, PL, SE, DK, FI, GR, PT (the 13 European countries with `china-europe-suez`). IN's `nearestRouteIds` reverts to `["india-europe", "india-se-asia", "gulf-asia-oil"]` — no more China-origin pollution. IN→DE now intersects on `india-europe` (only shared route), so the resolver picks it unambiguously. Test tightening (the actual lock the reviewer asked for): `tests/country-port-clusters-asia-europe-lane.test.mts` adds an `EXPECTED_PRIMARY_ROUTE_TO_DE` map and asserts `computeLane(<iso2>, 'DE', '85', 'container').primaryRouteId === expected` for all 12 Asian-port countries. IN→DE must equal `'india-europe'`; all others must equal `'china-europe-suez'`. Removed the weaker "noModeledLane: false for the matrix" test — the stricter primaryRouteId assertion subsumes it (a `noModeledLane=true` response fails the equality check with a clear message naming the offender). Bite-test of the new assertion: - Revert `india-europe` from DE only → both tests #1 and #3 flip red naming IN: data invariant says "IN → DE: shared routes = []", lane assertion says "IN → DE: noModeledLane=true (expected primaryRouteId='india-europe')". Confirms the assertion bites the exact reviewer-flagged regression. - HK→DE remains green (china-europe-suez is correct for an HK origin). - 38/38 of the existing `tests/route-explorer-lane.test.mts` smoke matrix stays green. tsc clean. The other Asian origins (TW/JP/KR/VN/TH/PH/SG/MY/ID) already correctly intersect DE on `china-europe-suez` (added in this PR's first commit) — that's the trunk route their containers actually take to Europe, so their primaryRouteId assertion was already correct. * test(route-explorer): drop redundant HK-specific case (PR #3832 review P2) Greptile P2: the standalone HK → DE test was fully subsumed by the matrix test, which iterates over ASIAN_PORT_COUNTRIES (includes HK at index 1) and asserts `primaryRouteId === EXPECTED_PRIMARY_ROUTE_TO_DE[iso2]`. HK's expected value is 'china-europe-suez'; a regression on HK fails the equality check with a clear message, just like every other origin. Renamed the surviving matrix test to call out HK→DE explicitly so the provenance trail to the pr-3718 reporter symptom isn't lost.
Two empty-on-mobile bug reports, one root cause: PRO-gated loaders silently strand when `hasPremiumAccess()` returns false at the boot tick.
Symptom 1 — `PREMIUM STOCK ANALYSIS` empty body
`data-loader.ts:456` gates `loadStockAnalysis` on `hasPremiumAccess() && shouldLoad(...)`. If Clerk/Convex haven't hydrated by the `loadAllData(true)` boot call, the task is skipped. `App.ts:1017-1031 firePremiumLoaders` only re-fired `loadTradePolicy` on Free→Pro — every other PRO-gated loader was on its own. The scheduled refresh that WOULD have caught up is gated to `SITE_VARIANT === 'finance'` (`App.ts:1495`), so on the full variant the panel sits empty for the entire session.
Secondary contributor: `Panel.ts:954` `showRetrying` and `Panel.ts:1045` `setContent` both `if (this._locked) return;` silently, so even if `loadStockAnalysis` DID run while the panel was rendering its locked CTA, the data render was swallowed; `unlockPanel` then restored the pre-lock `_savedContent` (an empty body). Fixing the fan-out covers this too — the re-fire happens AFTER unlock, when `_locked` is false.
Symptom 2 — Route Explorer "No modeled lane" for HK→DE (user is Pro)
`src/services/supply-chain/index.ts` had 8 fetchers gated by the bare `hasPremiumAccess()` (no auth state). The bare variant reads only the cached `isProUser()` Convex tier signal. `RouteExplorer.fetchLane:212` uses `hasPremiumAccess(getAuthState())` which ALSO accepts Clerk's `user.role === 'pro'`. The two diverge during the Clerk-resolved-but-Convex-stale window: RouteExplorer's outer gate passes, then `fetchRouteExplorerLane`'s inner gate fails and returns `{ ...emptyRouteExplorerLane, ...args }` — which has `noModeledLane: true`. The UI then renders "No modeled lane for this pair", the same message shown when the dataset genuinely lacks the lane. The user can't tell the two cases apart.
Audit blast-radius
```
$ rg -n "if \(!hasPremiumAccess\(\)\)" src/services/
src/services/supply-chain/index.ts — 8 fetchers (chokepointIndex, sectorDependency, routeExplorerLane, routeImpact, products, multiSectorShock, +2)
src/services/sanctions-pressure.ts — 1
src/services/economic/index.ts — 1
src/services/correlation-engine/engine.ts — 1
src/services/analysis-framework-store.ts — 1
```
This PR addresses the 8 in `supply-chain/index.ts` (where the misleading `noModeledLane` sentinel was reported). The other 4 are similar shape but their empty sentinels don't produce a misleading user-visible message in the same way — left for a follow-up rather than blowing up this PR's scope.
Fix
Prong 1 — expand `firePremiumLoaders` at `App.ts:1017-1031` to fan out every PRO-gated loader on Free→Pro: `loadStockAnalysis`, `loadStockBacktest`, `loadDailyMarketBrief`, `loadMarketImplications`, `loadWsbTickers`, `loadResilienceRanking` (plus the existing `loadTradePolicy`). Each loader is idempotent and self-checks `hasPremiumAccess()` internally, so re-firing is safe.
Prong 2 — close the Clerk/Convex divergence window in `src/services/supply-chain/index.ts`. Introduce a tiny `gatedPremium() = hasPremiumAccess(getAuthState())` helper and route all 8 supply-chain fetcher gates through it. Eliminates the divergence for every supply-chain consumer in one shot.
Prong 3 — audit-locking regression test at `tests/premium-loaders-fan-out-coverage.test.mts`. Statically extracts every `this.loadX()` call sitting inside an `if (hasPremiumAccess(...))` block in `data-loader.ts` and asserts each appears in `App.ts:firePremiumLoaders` (with a documented `ALLOWLIST` escape hatch). The next contributor who adds a PRO-gated loader without wiring fan-out gets a CI failure with a clear "add `void this.dataLoader.loadX();` to firePremiumLoaders" message.
Test plan