Discovery review fixes — 2026-09-19
Worktree: C:/Users/jefra/.codex/worktrees/yumina-discovery.
Scope: the five actionable findings in 2026-09-19-discovery-review.md. No new recommendation/ML features. The cursor rollout flag remains off by default. No commits, pushes, deploys, browser work, subagents, or production access were performed.
Outcome: all five fixes complete; full OSS verification passed with exit code 0. The controller subsequently reported that the original reviewer's scoped re-review closes all five findings and that the current hosted tree passes full typecheck/build.
Fixes and regression evidence
Mobile virtualization and mode transitions.
DiscoveryGridselects the document/window virtualizer when the existingisDocumentPageScrollercontract says the document owns scrolling. Its scroll margin is the grid's document position; desktop keeps element virtualization. A mutation observer watches the mobile scroll marker, covering ownership changes that happen after the resize event. Only the active owner has virtualization enabled. React's virtualizer flush is disabled to allow ownership transfers during layout without a nestedflushSyncwarning.- Before the fix, the new phone fixture mounted all 1,725 cards; the ownership transition fixture failed the same bound.
- After the fix, desktop/mobile fixtures mount between 1 and 99 cards, window scrolling changes the range and returning to the top restores the first card. The grid has a nonzero 900px document origin. Transitions work both ways, including marker changes after resize and events from the former owner.
OSS export boundary. The shared
/hubroute now checksedition.info().features.hubbefore authentication, rate limiting, ranking, or hosted-table access. The manifest excludes the hosted discovery implementation, its pure/HTTP tests, the hosted app pagination helpers/tests, and the embedding backfill test whose script is excluded. A localdiscovery-feedstub replaces the hosted transport; the route imports its error type through that same boundary. No candidate-retrieval exports were added to the recommendation stub. Shared published-content embedding code and its supported tests remain in the export.- Before the fix, the new local-edition route regression returned 503, querying missing hosted tables instead of returning 404. It now returns 404 for cursor, legacy recommended, and search requests even when
DISCOVERY_CURSOR_ENABLED=true, without setting a guest cookie. - The generated-tree regression first failed because
discovery-session.tswas exported. It now verifies that hosted discovery/session/Redis modules and tests are absent, the intended stub is present, and core embedding code/tests plus the local route test are retained. - Existing exporter pruning already removed the missing-script backfill test; explicit manifest exclusions make that boundary intentional and also cover indirect hosted HTTP tests.
- Before the fix, the new local-edition route regression returned 503, querying missing hosted tables instead of returning 404. It now returns 404 for cursor, legacy recommended, and search requests even when
Dedicated discovery IP limit. Discovery now uses
ipRateLimitMiddleware(180, 60, "discovery").- Before the fix, after 40 successful discovery requests, the first request through the actual auth router returned 429. Forty preceding feed-event requests also caused discovery request 141 to be rejected.
- After the fix, auth retains all 40 requests, rejecting request 41 with
Retry-After; discovery retains all 180 requests after event traffic, rejecting request 181. These tests mount the real Hono routers/middleware. Only the final auth handler is replaced with a harmless response; no auth provider is contacted.
Dismiss/undo profile invalidation. Both successful dismissal mutations await
invalidateRecommendationProfilealongside the existing engagement/feed invalidations and primary-read flag. The existing invalidator clears every scope: all, EN, ZH, ES, JA.- New tests run the actual recommendation loader, profile serialization/deserialization, SQL, ranking, auth middleware and dismissal route code. An in-memory Redis transport supplies deterministic cache reuse, and PGlite supplies the isolated database. Tests explicitly verify that the second profile load hits the serialized cache.
- Before the fix, dismiss left cached profiles able to rank both language candidates; undo left the exact-world/group exclusions cached. After the fix, every scope immediately excludes the dismissed group and immediately allows both candidates after undo.
Automatic pagination counts visible additions.
RecTabcomputes the unique appended rows, then applies the same current hidden/favorite/Library-group predicate used to render cards. Three consecutive pages adding zero visible cards stop automatic fetching. A visible unique addition resets the budget. The request's cursor/offset and manual continuation remain available. A ref reads the latest local exclusions without restarting the feed on each local mutation; budget updates occur outside React state updaters.- Before the fix, duplicate legacy pages made 10 automatic requests instead of four (one visible page plus three zero-addition pages). Hidden/favorited/Library-sibling pages made 6 requests instead of four. The visible-reset case made 9 instead of seven.
- Component regressions now cover those cases, deduplicated card counts, preserved manual offsets/cursors, a manual request after the budget is spent, and resetting the budget when a unique visible card arrives.
Validation
- App:
pnpm exec tsx --tsconfig tsconfig.app.json --test src/features/hub/discovery-grid.test.ts src/features/hub/rec-tab.test.ts src/lib/discovery-pagination.test.ts src/lib/page-scroll.test.ts(frompackages/app) — 14 passed. DOM tests use jsdom, not a browser. - Server integration:
node packages/server/scripts/test-local.mjs src/routes/discovery-feed.test.ts src/routes/feed-dismiss-profile.test.ts src/routes/worlds-local-discovery.test.ts— 6 passed, including the retained 713-story HTTP traversal/replay/guest-ownership test. New cache integration test is registered with the existing full-schema local bootstrap. - Related server behavior:
node packages/server/scripts/test-local.mjs src/lib/discovery-pagination.test.ts src/lib/discovery-session.test.ts src/lib/discovery-eligibility.test.ts src/middleware/rate-limit.test.ts src/middleware/rate-limit-failover.test.ts— 21 passed. - Generated export contract:
node --test scripts/oss-discovery-boundary.test.mjs— 1 passed against the generated tracked tree. - Hosted server and app typechecks — passed. Hosted app typecheck initially caught readonly assignments in the new DOM fixture; those were corrected and the final app typecheck plus all 14 focused app tests passed without warnings.
- Edition boundary:
pnpm --filter @yumina/app check:edition— passed, no core imports of hosted paths. - Full OSS verification:
node scripts/oss-export.mjs --verify— passed, exit code 0,[oss-export] verify OK. Installation, uncached typecheck, build and all exported test tasks passed. The export selected 1,508 files from 3,217 tracked files and scanned 1,432 text files against the 19 denylist patterns. Generated artifacts remain in ignoredoss/out; no generated source changes were retained in the worktree. Log:oss-discovery-verify.log(ignored local artifact).
| Exported suite | Passed | Failed | Skipped |
|---|---|---|---|
| Shared | 45 | 0 | 0 |
| Engine | 985 | 0 | 0 |
| App | 728 | 0 | 0 |
| Server | 1,296 | 0 | 2 |
| Total | 3,054 | 0 | 2 |
The two existing server skips are the Redis-backed browse-engagement and play-engagement integration tests, which require ANALYTICS_TEST_REDIS_URL. The isolated launcher intentionally does not forward that external-service setting. The retained local-edition discovery route test ran and passed in the actual exported tree.
Files changed by this fix wave
packages/app/src/features/hub/discovery-grid.tsxpackages/app/src/features/hub/discovery-grid.test.tspackages/app/src/features/hub/rec-tab.tsxpackages/app/src/features/hub/rec-tab.test.tspackages/server/src/routes/worlds.tspackages/server/src/routes/discovery-feed.test.tspackages/server/src/routes/feed-events.tspackages/server/src/routes/feed-dismiss-profile.test.tspackages/server/src/routes/worlds-local-discovery.test.tspackages/server/src/lib/discovery-feed.ts(error re-export for the transport boundary)packages/server/src/lib/recommendations.ts(invalidation documentation)packages/server/scripts/test-local-environment.mjsoss/manifest.jsonoss/stubs/server/lib/discovery-feed.tsscripts/oss-discovery-boundary.test.mjs- This evidence report.
Staging and remaining verification boundaries
The exporter enumerates git ls-files and copies current working-tree contents. At the controller's explicit direction, prospective new source/test files from the initial discovery and embedding work were staged together with this wave's exact source/test/manifest/stub fixes. Existing modified discovery/embedding sources were staged as well. No commit was created. Controller docs and packages/server/scripts/benchmark-discovery.ts were not staged; the benchmark is covered by the manifest's existing server-script exclusion and forbidden-path rule and is checked absent in the generated-tree regression. This report is intentionally unstaged.
This wave verifies behavior with jsdom and isolated local PGlite, plus an in-memory Redis profile transport. It does not measure mobile browser layout, actual shared Redis operations, admission/session memory limits or concurrent origin load. The documented Redis/admission/concurrency gate remains required before enabling the cursor rollout; the controller's revised benchmark is not changed or reinterpreted here.
The controller owns integration with the two subsequently advanced upstream commits and re-verification of that combined tree. This OSS result applies to the current staged fix tree before that integration. No rebase or main integration was attempted, and no further code changes were needed after the scoped re-review.
