#57

Create GPT portfolio variant

Openstylesshwants to merge intofrom

No description provided.

Activity

8
mikeyreviewed2mo ago
mikey2mo ago

Review summary

Reviewed app/gpt/page.tsx (the only changed file, +140/-108, purely presentational). No backend, auth, data, or schema surface is touched.

Checked:

  • New imports (cn from @/lib/utils, type PortfolioProject from @/lib/portfolio-content) resolve correctly — verified both exist and are exported as expected.
  • getLatestCommitDate() now scopes git log to app/gpt/page.tsx lib/portfolio-content.ts instead of the whole repo — narrower and more accurate for "latest modified," no behavior regression (same try/catch fallback to null → "Unavailable").
  • ProjectContactSheet reads project.deckImages.slice(0, 3); confirmed all three entries in CANONICAL_PORTFOLIO (lib/portfolio-content.ts) currently have exactly 3 deckImages, so no images are silently dropped today. Worth knowing this variant now caps the deck preview at 3 images if that list ever grows, unlike the previous version which rendered all of them — likely intentional given the new 3-column contact-sheet layout, but flagging since it's a silent cap rather than a .map over the full array.
  • Layout/style-only changes elsewhere (max-width, spacing, cn() class merging, new footer) look consistent and self-contained to this route.

No blocking correctness, security, or reliability issues found. This is an isolated, cosmetic redesign of an unindexed (robots: noindex) alternate portfolio route with no shared-state or data-model impact.

Non-blocking note: the deckImages.slice(0, 3) cap mentioned above — no action needed unless a project is later given more than 3 images and all of them are expected to show.

mikeyreviewed2mo ago
mikey2mo ago

Review: low-risk styling/layout refactor of app/gpt/page.tsx. No blocking correctness, security, or data-safety issues found.

What I checked:

  • getLatestCommitDate() still runs a hardcoded execSync string (now scoped to app/gpt/page.tsx lib/portfolio-content.ts) — no user input reaches the shell, so no injection risk; the added path scoping is a correctness improvement (freshness date now reflects only the files that actually affect this page).
  • PortfolioProject/cn imports resolve against lib/portfolio-content.ts and lib/utils.ts on main — types line up.
  • New ProjectContactSheet/LatestModified components are pure presentational extractions; JSX restructuring (header grid, footer, section ids/aria-labelledby pairs) stays internally consistent.

Non-blocking note:

  • ProjectContactSheet now does project.deckImages.slice(0, 3) to match the new 3-column contact-sheet layout. All current entries in CANONICAL_PORTFOLIO have exactly 3 deckImages, so this is safe today, but it will silently drop images if a future project entry has more than 3 — worth a comment or a length assertion if that data is expected to grow.

No test coverage changes — consistent with this repo's existing pattern of untested static variant pages, not a regression introduced by this PR.

stylesshreviewed29d ago
stylesshreviewed29d ago
stylesshapproved changes28d ago
stylesshOwner28d ago

Clean refresh. Pulling out ProjectContactSheet and LatestModified makes the page easier to read, and switching to cn() is a nice cleanup. One thing to check before merging: the path-scoped git log can return nothing on a shallow build clone and show "Unavailable". Please confirm the date renders on a preview deploy, or fall back to the unscoped query when the output is empty.

stylesshapproved changes21d ago
stylesshOwner21d ago

Clean refresh. Pulling out ProjectContactSheet and LatestModified makes the page easier to read, and switching to cn() is a nice cleanup. One thing to check before merging: the path-scoped git log can return nothing on a shallow build clone and show "Unavailable". Please confirm the date renders on a preview deploy, or fall back to the unscoped query when the output is empty.