Skip to content

JMCP: Papers - jmcp/search-and-discovery-skeleton-SU6U3feE#4

Closed
landigf wants to merge 1 commit into
mainfrom
jmcp/search-and-discovery-skeleton-SU6U3feE
Closed

JMCP: Papers - jmcp/search-and-discovery-skeleton-SU6U3feE#4
landigf wants to merge 1 commit into
mainfrom
jmcp/search-and-discovery-skeleton-SU6U3feE

Conversation

@landigf

@landigf landigf commented Mar 23, 2026

Copy link
Copy Markdown
Owner

Review: Search & Discovery Skeleton

Change summary

Layer What changed
Contracts New Zod schemas: searchInputSchema, searchResultSetSchema, discoverTopicGroupSchema, discoverSectionsSchema with matching TS types
DB / Repository Two new methods on PapersRepository interface — search() and getDiscoverSections() — implemented in DemoRepository with in-memory substring matching
Web — routes /search (query-param ?q=) and /discover pages, both server-rendered RSC pages
Web — nav Two new <Link>s added to SiteHeader

Total unstaged delta: +151 lines across 3 files, plus 2 new page files (~220 lines).

Validation confidence: High

  • Contracts are well-typed; z.infer derives types from schemas — no drift risk.
  • Repository methods follow the same pattern as every existing method (read state, filter, map, sort). Consistent style.
  • Pages are straightforward RSC renders with no client state; search uses native <form method="GET">, so it works without JS.
  • Cross-linking between /search and /discover is correct (/search?q=..., /discover).

Risks that still remain

Risk Severity Note
No server action / validation for search input Low searchInputSchema is declared but never called — the page reads searchParams.q raw. Fine for demo, but the schema max-length (200) isn't enforced at the boundary.
O(n) full-scan search Low Substring .includes() over all papers/users/conferences. Acceptable for demo data; won't scale to real volume without an index.
getDiscoverSections viewer fallback Low When no viewer is resolved, crossDisciplinaryPicks degenerates to top-scored papers (no interests to exclude). Intentional but worth documenting.
No loading/error UI Low Both pages are fully server-rendered with no loading.tsx or error.tsx — a slow demo-store read would show a blank page.
Contracts test coverage Info Existing contracts/test/index.test.ts doesn't cover the new schemas yet.

No blocking issues. Good to commit.

@landigf

landigf commented Mar 25, 2026

Copy link
Copy Markdown
Owner Author

Closing: will integrate valuable features directly on main to avoid cascade merge conflicts.

@landigf landigf closed this Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant