tech-debt: dedupe helpers, surface real errors, prune pass-throughs#111
Merged
Conversation
…ANGELOG Drops three exploratory artifacts that were committed to the repo root and never referenced (`straude-codemap.html`, `prometheus-list.png`, `posthog-setup-report.md`). Documents the parallel tech-debt cleanup landing across worktrees in `docs/CHANGELOG.md` under Unreleased. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s and api routes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… with tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lpers Pull together four sets of duplicated web-only helpers so each pattern has a single source of truth. - New `apps/web/lib/utils/dates.ts` exports `formatDateMonDay` (admin chart axes) and `formatDateKey` (local-time YYYY-MM-DD). Replaces 6 inline copies in admin charts and 5 inline copies in share-assets / contribution / recap utilities. - New `fillContributionDays` export on `apps/web/lib/utils/recap.ts` replaces three byte-identical copies in the recap page, RecapCard, and recap-image renderer. - New `apps/web/components/app/shared/useFocusTrap.ts` hook replaces three nearly-identical inline focus traps in SubmitPromptWidget, ImageLightbox, and SuggestCompanyWidget. The shared selector matches the broader form-aware variant; the lightbox has no textareas/inputs so the wider selector is a no-op there. - New `apps/web/scripts/_lib.ts` exports `ensureCleanOutputDir(dir)` used by all three OG generator scripts. No behavior change. tsc + apps/web vitest pass.
…pers Removes three files surfaced by the dead-code sweep with zero importers: - apps/web/lib/performance/interaction.ts (entire perf dir was empty after) - apps/web/components/landing/GlobalFeed.tsx (CHANGELOG had announced its removal months ago, but the file itself was never deleted) - packages/cli/src/lib/codex.ts (7-line re-export shim no one imported) Also downgrades 8 stray exports to module-private — each is referenced only inside its own file. The export keyword was misleading scope: - enrichComments (apps/web/lib/comments.ts) - OPEN_STATS_REVALIDATE_SECONDS (apps/web/lib/open-stats.ts) - extractPublicStoragePath (apps/web/lib/storage.ts) - isThemePreference (apps/web/lib/theme.ts) - buildProfileShareText (apps/web/lib/utils/profile-share.ts) - getMissingSupabaseServerEnv, hasSupabaseBrowserEnv, hasSupabaseServerEnv (apps/web/lib/supabase/env.ts) CHANGELOG entries extended. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… dead OPEN_STATS_REVALIDATE_SECONDS was declared but never wired into a revalidate export. hasSupabaseBrowserEnv and hasSupabaseServerEnv were boolean wrappers around getMissingSupabase*Env that no caller used. ESLint flagged all three as unused once the export keyword no longer shielded them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR centralizes shared helpers into a new ChangesShared package & re-exports
Web utilities & date helpers
Replace local implementations with shared imports
Focus-trap adoption
Recap / contribution flow updates
Export visibility & API surface changes
Dead code removal
Scripts, build, and config tweaks
Error handling & logging
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apps/web/lib/utils/recap.ts (1)
117-121: ⚡ Quick winUse
formatDateKey(d)instead of the inline template literal for consistency.Line 120 re-inlines the same
YYYY-MM-DDtemplate literal that the PR explicitly centralised intoformatDateKey. The output is identical today, but the inline copy is exactly the kind of divergence this PR was created to prevent.♻️ Proposed fix
+import { formatDateKey } from "@/lib/utils/dates"; ... for (let i = 0; i < cappedDays; i++) { const d = new Date(start); d.setDate(start.getDate() + i); - const key = `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, "0")}-${String(d.getDate()).padStart(2, "0")}`; + const key = formatDateKey(d); result.push({ date: key, cost_usd: lookup.get(key) ?? 0 }); }(
formatDateKeyis already imported at line 2 — no new import needed.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/lib/utils/recap.ts` around lines 117 - 121, Replace the inline YYYY-MM-DD template literal with the centralized helper: after creating d in the loop (used with start and cappedDays), call formatDateKey(d) to compute key instead of the template; then push { date: key, cost_usd: lookup.get(key) ?? 0 } into result (symbols to edit: formatDateKey, d, lookup, result, start, cappedDays).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/api/embed/`[username]/svg/route.ts:
- Line 2: Add an explicit rootDir to the shared package TS config so tsc can
compute output layout: open the packages/shared/tsconfig.json and set "rootDir"
to the source root (e.g., "src") alongside existing compilerOptions so
declaration files are emitted and imports like formatTokens (imported in
apps/web/app/api/embed/[username]/svg/route.ts) resolve; after adding rootDir,
rebuild to ensure TS5011 and downstream TS2307 errors are fixed.
In `@apps/web/components/app/shared/useFocusTrap.ts`:
- Around line 26-30: The focusable NodeList from
containerRef.current.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR) can
include disabled elements (e.g., a disabled submit button), so computing
first/last directly can pick a non-tabbable element and allow focus to escape;
filter the NodeList to only include elements that are not disabled and are
actually tabbable (e.g., check !elem.hasAttribute('disabled') and elem.tabIndex
!== -1 or use elem.matches(':not([disabled])')) before deriving first and last
(references: containerRef, FOCUSABLE_SELECTOR, first, last) so Tab wrapping uses
the real tabbable elements.
In `@apps/web/lib/utils/post-share.ts`:
- Around line 2-5: The TypeScript errors come from importing subpaths from
`@straude/shared` (e.g., getShareModelLabel, prettifyModel in
apps/web/lib/utils/post-share.ts and format/formatCurrency in
apps/web/lib/utils/format.ts) before the package has been compiled to its dist/
output declared in package.json exports; fix by running the build for the shared
package (run npm run build or pnpm build in packages/shared/) to produce the
dist/*.js and dist/*.d.ts files, then ensure your CI/monorepo pipeline builds
packages/shared before apps/web (or adjust workspace build order) so those
subpath imports resolve correctly.
In `@apps/web/lib/utils/recap.ts`:
- Around line 98-114: The week-branch DST bug occurs because start is created
with new Date(now) (preserving wall-clock time) while now may be in DST, causing
daysSinceStart to undercount; to fix, normalize both start and now to local
midnight before computing daysSinceStart and cappedDays (e.g., set
hours/minutes/seconds/ms to 0 for start and now) inside the code paths that
compute msPerDay, daysSinceStart and cappedDays so the diff uses whole days;
update the variables used in the calculation (now, start, msPerDay,
daysSinceStart, cappedDays) and ensure the month branch behavior remains
unchanged.
In `@docs/CHANGELOG.md`:
- Around line 59-65: There are two "### Removed" blocks under the "##
Unreleased" version; remove the duplicate block you added and append its bullet
entries (the items about stale root-level artifacts, pass-through wrappers,
orphaned source files, stray `export` keywords, and truly-dead
constants/helpers) into the existing "### Removed" section within the same "##
Unreleased" block so the changelog follows Keep a Changelog (single header per
section); locate the duplicate `### Removed` header you added and merge its
bullet lines into the existing `### Removed` section under `## Unreleased`, then
delete the duplicate header and its empty space.
In `@packages/shared/src/format.ts`:
- Line 4: The check that formats thousands currently returns `${(n /
1_000).toFixed(1)}k`, which produces "1.0k" for exact thousands; change this to
strip trailing ".0" by converting the toFixed string back to a number before
appending "k" (e.g., use parseFloat((n / 1_000).toFixed(1)) or Number(...) to
drop the unnecessary ".0"). Update the expression in the same conditional that
contains `if (n >= 1_000) return \`${(n / 1_000).toFixed(1)}k\`;` so outputs
become "1k", "1.4k", "1.5k", etc., without altering the fractional precision for
non-exact thousands.
In `@packages/shared/src/models.ts`:
- Around line 14-15: The pattern matching on the normalized model ID incorrectly
collapses any string starting with "o3" or "o4" to the family name; update the
checks around the normalized variable so they only match the bare family or
family followed by a hyphen (e.g., use /^o4(?:$|-)/i and /^o3(?:$|-)/i) to
preserve suffixes like "o4-mini" or "o3-mini" rather than rewriting them to just
"o4"/"o3". Locate the two regex lines that reference normalized and replace them
with the stricter family-or-hyphen matches so suffixes are kept intact.
---
Nitpick comments:
In `@apps/web/lib/utils/recap.ts`:
- Around line 117-121: Replace the inline YYYY-MM-DD template literal with the
centralized helper: after creating d in the loop (used with start and
cappedDays), call formatDateKey(d) to compute key instead of the template; then
push { date: key, cost_usd: lookup.get(key) ?? 0 } into result (symbols to edit:
formatDateKey, d, lookup, result, start, cappedDays).
🪄 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
Run ID: 089b7c6c-dea1-418b-80df-ace1982407a2
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockprometheus-list.pngis excluded by!**/*.png
📒 Files selected for processing (65)
apps/web/__tests__/lib/strip-markdown.test.tsapps/web/app/admin/components/CodexGrowthCharts.tsxapps/web/app/admin/components/GrowthMetrics.tsxapps/web/app/admin/components/ModelShareChart.tsxapps/web/app/admin/components/ModelUsageChart.tsxapps/web/app/admin/components/NorthStarChart.tsxapps/web/app/admin/components/UserSignupsChart.tsxapps/web/app/api/embed/[username]/svg/route.tsapps/web/app/api/users/me/route.tsapps/web/app/recap/[username]/page.tsxapps/web/components/app/feed/ActivityCard.tsxapps/web/components/app/profile/ContributionGraph.tsxapps/web/components/app/profile/ProfileSharePanel.tsxapps/web/components/app/prompts/SubmitPromptWidget.tsxapps/web/components/app/recap/RecapCard.tsxapps/web/components/app/shared/ImageLightbox.tsxapps/web/components/app/shared/useFocusTrap.tsapps/web/components/app/token-rich/SuggestCompanyWidget.tsxapps/web/components/landing/GlobalFeed.tsxapps/web/lib/admin.tsapps/web/lib/api/cli-auth.tsapps/web/lib/comments.tsapps/web/lib/email/notification-email.tsxapps/web/lib/email/send-comment-email.tsapps/web/lib/open-stats.tsapps/web/lib/performance/interaction.tsapps/web/lib/share-assets/github-card-data.tsapps/web/lib/share-assets/heatmap.tsapps/web/lib/share-assets/post-card-image.tsxapps/web/lib/share-assets/profile-card-data.tsapps/web/lib/storage.tsapps/web/lib/supabase/env.tsapps/web/lib/theme.tsapps/web/lib/utils/dates.tsapps/web/lib/utils/format.tsapps/web/lib/utils/post-share.tsapps/web/lib/utils/profile-share.tsapps/web/lib/utils/recap-image.tsxapps/web/lib/utils/recap.tsapps/web/lib/utils/strip-markdown.tsapps/web/package.jsonapps/web/proxy.tsapps/web/scripts/_lib.tsapps/web/scripts/generate-og-athletic-surge.tsapps/web/scripts/generate-og-real-users.tsapps/web/scripts/generate-og.tsdocs/CHANGELOG.mdpackages/cli/__tests__/codex.test.tspackages/cli/__tests__/commands/push.test.tspackages/cli/__tests__/flows/cli-sync-flow.test.tspackages/cli/package.jsonpackages/cli/src/commands/login.tspackages/cli/src/commands/push.tspackages/cli/src/components/ModelPalette.tsxpackages/cli/src/components/PushSummary.tsxpackages/cli/src/lib/ccusage.tspackages/cli/src/lib/codex-native.tspackages/cli/src/lib/codex.tspackages/shared/package.jsonpackages/shared/src/format.tspackages/shared/src/index.tspackages/shared/src/models.tspackages/shared/tsconfig.jsonposthog-setup-report.mdstraude-codemap.html
💤 Files with no reviewable changes (5)
- packages/cli/src/lib/codex.ts
- posthog-setup-report.md
- apps/web/lib/performance/interaction.ts
- straude-codemap.html
- apps/web/components/landing/GlobalFeed.tsx
| @@ -1,4 +1,5 @@ | |||
| import { NextRequest } from "next/server"; | |||
| import { formatTokens } from "@straude/shared/format"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check packages/shared/package.json for exports/types configuration
fd -g 'package.json' packages/shared --exec cat {}
# Verify TypeScript compiler can resolve subpath from apps/web tsconfig
fd -g 'tsconfig*.json' apps/web --exec cat {}
# Check if any declarations are already generated
fd -e d.ts packages/sharedRepository: ohong/straude
Length of output: 1929
🏁 Script executed:
# Check if dist/format.d.ts exists
fd -type f packages/shared/dist
# Check if source files exist
fd -type f packages/shared/src
# Look at the directory structure of packages/shared
tree -L 3 packages/shared 2>/dev/null || find packages/shared -type f -o -type d | head -30
# Check if there's a build output or tsconfig in packages/shared
cat packages/shared/tsconfig.json 2>/dev/null || echo "No tsconfig.json found"Repository: ohong/straude
Length of output: 846
🏁 Script executed:
# Check if dist is in .gitignore
cat packages/shared/.gitignore 2>/dev/null || echo "No .gitignore in packages/shared"
cat .gitignore 2>/dev/null | grep -E "dist|node_modules" || echo "Checking root .gitignore"
# Check git status to see if dist is untracked or ignored
git -C packages/shared status 2>/dev/null || git status packages/shared
# Verify the exports field one more time from the initial results is accurate
cat packages/shared/package.json | grep -A 15 '"exports"'Repository: ohong/straude
Length of output: 720
🏁 Script executed:
cd packages/shared && npm run buildRepository: ohong/straude
Length of output: 637
Actual blocker: packages/shared/tsconfig.json is missing rootDir configuration.
The build fails with TS5011:
The 'rootDir' setting must be explicitly set to this or another path to adjust your output's file layout.
The exports field in package.json is already correctly configured. The issue is that packages/shared/tsconfig.json needs an explicit rootDir setting:
"compilerOptions": {
"target": "ES2022",
"module": "ESNext",
"moduleResolution": "Bundler",
"strict": true,
"esModuleInterop": true,
"skipLibCheck": true,
"isolatedModules": true,
"declaration": true,
"outDir": "./dist",
+ "rootDir": "./src"
},Without this, the TypeScript compiler cannot build the package, so the declaration files are never generated, causing the TS2307 error in consuming packages.
🧰 Tools
🪛 GitHub Actions: CI
[error] 2-2: TypeScript (TS2307): Cannot find module '@straude/shared/format' or its corresponding type declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/embed/`[username]/svg/route.ts at line 2, Add an explicit
rootDir to the shared package TS config so tsc can compute output layout: open
the packages/shared/tsconfig.json and set "rootDir" to the source root (e.g.,
"src") alongside existing compilerOptions so declaration files are emitted and
imports like formatTokens (imported in
apps/web/app/api/embed/[username]/svg/route.ts) resolve; after adding rootDir,
rebuild to ensure TS5011 and downstream TS2307 errors are fixed.
Comment on lines
+26
to
+30
| const focusable = | ||
| containerRef.current.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR); | ||
| if (focusable.length === 0) return; | ||
| const first = focusable[0]; | ||
| const last = focusable[focusable.length - 1]; |
There was a problem hiding this comment.
Filter disabled elements before computing first/last.
Both modal-form consumers can disable their submit button while the dialog stays open, so this list can pick a disabled control as last. When that happens, Tab on the real last tabbable element falls through and focus escapes the modal.
Suggested fix
- const focusable =
- containerRef.current.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR);
+ const focusable = Array.from(
+ containerRef.current.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR),
+ ).filter((el) => !el.matches(":disabled"));
if (focusable.length === 0) return;
const first = focusable[0];
const last = focusable[focusable.length - 1];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/components/app/shared/useFocusTrap.ts` around lines 26 - 30, The
focusable NodeList from
containerRef.current.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR) can
include disabled elements (e.g., a disabled submit button), so computing
first/last directly can pick a non-tabbable element and allow focus to escape;
filter the NodeList to only include elements that are not disabled and are
actually tabbable (e.g., check !elem.hasAttribute('disabled') and elem.tabIndex
!== -1 or use elem.matches(':not([disabled])')) before deriving first and last
(references: containerRef, FOCUSABLE_SELECTOR, first, last) so Tab wrapping uses
the real tabbable elements.
Comment on lines
+2
to
+5
| import { getShareModelLabel } from "@straude/shared/models"; | ||
| import { formatCurrency, formatTokens } from "./format"; | ||
|
|
||
| export { prettifyModel, getShareModelLabel } from "@straude/shared/models"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect shared package configuration and tsconfig path aliases
echo "=== packages/shared/package.json ==="
fd "package.json" packages/shared --max-depth 1 --exec cat {}
echo ""
echo "=== apps/web/tsconfig.json ==="
fd "tsconfig.json" apps/web --max-depth 1 --exec cat {}
echo ""
echo "=== Source files in packages/shared/src ==="
fd . packages/shared/src --type f
echo ""
echo "=== Root tsconfig (for workspace paths) ==="
fd "tsconfig.json" . --max-depth 1 --exec cat {}Repository: ohong/straude
Length of output: 1681
🏁 Script executed:
#!/bin/bash
# Check if dist folder exists and its contents
echo "=== packages/shared/dist (if exists) ==="
if [ -d "packages/shared/dist" ]; then
find packages/shared/dist -type f | head -20
else
echo "dist folder does not exist"
fi
echo ""
echo "=== packages/shared directory structure ==="
ls -la packages/shared/Repository: ohong/straude
Length of output: 454
CI blocker: @straude/shared subpath imports fail because the package has not been built.
The TS2307 errors occur in:
apps/web/lib/utils/post-share.tslines 2, 5 (@straude/shared/models)apps/web/components/app/feed/ActivityCard.tsxlines 10–11 (@straude/shared/models)apps/web/lib/open-stats.tsline 1 (@straude/shared/models)apps/web/lib/utils/format.tsline 1 (@straude/shared/format)
Root cause: packages/shared/dist/ does not exist. The package source files are present (src/models.ts, src/format.ts, src/index.ts), but they have not been compiled to the dist/ directory that package.json's exports map declares:
"exports": {
"./models": { "types": "./dist/models.d.ts", ... },
"./format": { "types": "./dist/format.d.ts", ... }
}TypeScript cannot resolve these imports because the declared .d.ts files don't exist yet.
Solution: Run npm run build (or pnpm build) in packages/shared/ to generate the dist/ directory with compiled output. Ensure the shared package is built before apps/web in the monorepo build pipeline.
🧰 Tools
🪛 GitHub Actions: CI
[error] 2-2: TypeScript (TS2307): Cannot find module '@straude/shared/models' or its corresponding type declarations.
[error] 5-5: TypeScript (TS2307): Cannot find module '@straude/shared/models' or its corresponding type declarations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/utils/post-share.ts` around lines 2 - 5, The TypeScript errors
come from importing subpaths from `@straude/shared` (e.g., getShareModelLabel,
prettifyModel in apps/web/lib/utils/post-share.ts and format/formatCurrency in
apps/web/lib/utils/format.ts) before the package has been compiled to its dist/
output declared in package.json exports; fix by running the build for the shared
package (run npm run build or pnpm build in packages/shared/) to produce the
dist/*.js and dist/*.d.ts files, then ensure your CI/monorepo pipeline builds
packages/shared before apps/web (or adjust workspace build order) so those
subpath imports resolve correctly.
Comment on lines
+98
to
+114
| const now = new Date(); | ||
| let start: Date; | ||
|
|
||
| if (period === "week") { | ||
| const day = now.getDay(); | ||
| const mondayOffset = day === 0 ? -6 : 1 - day; | ||
| start = new Date(now); | ||
| start.setDate(now.getDate() + mondayOffset); | ||
| } else { | ||
| start = new Date(now.getFullYear(), now.getMonth(), 1); | ||
| } | ||
|
|
||
| // Cap at today — don't render future days | ||
| const msPerDay = 86400000; | ||
| const daysSinceStart = | ||
| Math.floor((now.getTime() - start.getTime()) / msPerDay) + 1; | ||
| const cappedDays = Math.min(totalDays, daysSinceStart); |
There was a problem hiding this comment.
DST edge case causes week strip to show 6 days instead of 7 on spring-forward Sunday.
For the week branch, start = new Date(now) copies now's wall-clock time. When a spring-forward DST boundary falls between Monday and today (Sunday), start (Monday in standard time) and now (Sunday in daylight time) differ by 23 hours rather than 6 full days. Math.floor(23h × ... / 86_400_000) = 5, so daysSinceStart = 6 and the Sunday cell is silently omitted from the strip.
The month branch is unaffected because new Date(year, month, 1) creates start at local midnight regardless.
🐛 Proposed fix — normalize both dates to midnight before computing the diff
// Cap at today — don't render future days
const msPerDay = 86400000;
- const daysSinceStart =
- Math.floor((now.getTime() - start.getTime()) / msPerDay) + 1;
+ const todayMidnight = new Date(now);
+ todayMidnight.setHours(0, 0, 0, 0);
+ const startMidnight = new Date(start);
+ startMidnight.setHours(0, 0, 0, 0);
+ const daysSinceStart =
+ Math.floor((todayMidnight.getTime() - startMidnight.getTime()) / msPerDay) + 1;
const cappedDays = Math.min(totalDays, daysSinceStart);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const now = new Date(); | |
| let start: Date; | |
| if (period === "week") { | |
| const day = now.getDay(); | |
| const mondayOffset = day === 0 ? -6 : 1 - day; | |
| start = new Date(now); | |
| start.setDate(now.getDate() + mondayOffset); | |
| } else { | |
| start = new Date(now.getFullYear(), now.getMonth(), 1); | |
| } | |
| // Cap at today — don't render future days | |
| const msPerDay = 86400000; | |
| const daysSinceStart = | |
| Math.floor((now.getTime() - start.getTime()) / msPerDay) + 1; | |
| const cappedDays = Math.min(totalDays, daysSinceStart); | |
| const now = new Date(); | |
| let start: Date; | |
| if (period === "week") { | |
| const day = now.getDay(); | |
| const mondayOffset = day === 0 ? -6 : 1 - day; | |
| start = new Date(now); | |
| start.setDate(now.getDate() + mondayOffset); | |
| } else { | |
| start = new Date(now.getFullYear(), now.getMonth(), 1); | |
| } | |
| // Cap at today — don't render future days | |
| const msPerDay = 86400000; | |
| const todayMidnight = new Date(now); | |
| todayMidnight.setHours(0, 0, 0, 0); | |
| const startMidnight = new Date(start); | |
| startMidnight.setHours(0, 0, 0, 0); | |
| const daysSinceStart = | |
| Math.floor((todayMidnight.getTime() - startMidnight.getTime()) / msPerDay) + 1; | |
| const cappedDays = Math.min(totalDays, daysSinceStart); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/lib/utils/recap.ts` around lines 98 - 114, The week-branch DST bug
occurs because start is created with new Date(now) (preserving wall-clock time)
while now may be in DST, causing daysSinceStart to undercount; to fix, normalize
both start and now to local midnight before computing daysSinceStart and
cappedDays (e.g., set hours/minutes/seconds/ms to 0 for start and now) inside
the code paths that compute msPerDay, daysSinceStart and cappedDays so the diff
uses whole days; update the variables used in the calculation (now, start,
msPerDay, daysSinceStart, cappedDays) and ensure the month branch behavior
remains unchanged.
Comment on lines
+59
to
+65
| ### Removed | ||
|
|
||
| - **Stale root-level artifacts.** `straude-codemap.html`, `prometheus-list.png`, and `posthog-setup-report.md` were committed to the repo root during exploratory work and never referenced — deleted. | ||
| - **Pass-through wrappers left over from prior renames.** `proxy` is now a direct re-export of `updateSession`, `getCellColor` is a re-export of `getHeatmapCellColor`, and the unused `buildSubject`, `buildProfileShareUrl`, and `hasCodexLogs` shims were removed. | ||
| - **Orphaned source files.** `apps/web/lib/performance/interaction.ts` (zero importers, no `lib/performance` consumers remained), `apps/web/components/landing/GlobalFeed.tsx` (the CHANGELOG had previously announced its removal but the file was never deleted), and `packages/cli/src/lib/codex.ts` (a 7-line re-export shim with no callers — consumers now import from `./codex-native` directly) were deleted. | ||
| - **Stray `export` keywords on module-private helpers.** `enrichComments`, `extractPublicStoragePath`, `isThemePreference`, `buildProfileShareText`, and `getMissingSupabaseServerEnv` were exported but only referenced inside their own files. Downgraded to module-private to signal scope honestly. | ||
| - **Truly-dead constants and helpers.** Un-exporting also revealed three symbols with zero callers anywhere — `OPEN_STATS_REVALIDATE_SECONDS` (declared but never wired into a `revalidate` site), `hasSupabaseBrowserEnv`, and `hasSupabaseServerEnv`. Removed. |
There was a problem hiding this comment.
Merge into the existing ### Removed section instead of creating a duplicate.
A ### Removed section already exists at line 73 under ## Unreleased. Adding a second one violates Keep a Changelog format, which requires each section type to appear at most once per version block.
🛠️ Proposed fix — move entries to existing section
Remove the new ### Removed block (lines 59-65) and append its entries into the existing ### Removed block at line 73 instead:
-### Removed
-
-- **Stale root-level artifacts.** ...
-- **Pass-through wrappers ...** ...
-- **Orphaned source files.** ...
-- **Stray `export` keywords ...** ...
-- **Truly-dead constants ...** ...
-
### Added
- **User Signups bar chart ...** ...
...
### Removed
+- **Stale root-level artifacts.** ...
+- **Pass-through wrappers ...** ...
+- **Orphaned source files.** ...
+- **Stray `export` keywords ...** ...
+- **Truly-dead constants ...** ...
+
- **Product Hunt launch banner and badge.** ...As per coding guidelines: "follow Keep a Changelog format with Added/Changed/Fixed/Removed sections under ## Unreleased".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/CHANGELOG.md` around lines 59 - 65, There are two "### Removed" blocks
under the "## Unreleased" version; remove the duplicate block you added and
append its bullet entries (the items about stale root-level artifacts,
pass-through wrappers, orphaned source files, stray `export` keywords, and
truly-dead constants/helpers) into the existing "### Removed" section within the
same "## Unreleased" block so the changelog follows Keep a Changelog (single
header per section); locate the duplicate `### Removed` header you added and
merge its bullet lines into the existing `### Removed` section under `##
Unreleased`, then delete the duplicate header and its empty space.
| export function formatTokens(n: number): string { | ||
| if (n >= 1_000_000_000) return `${(n / 1_000_000_000).toFixed(1)}B`; | ||
| if (n >= 1_000_000) return `${(n / 1_000_000).toFixed(1)}M`; | ||
| if (n >= 1_000) return `${(n / 1_000).toFixed(1)}k`; |
There was a problem hiding this comment.
Undocumented behavioral change: round thousands now render as "1.0k" instead of "1k".
The old CLI implementation used Math.round(n / 1_000)k, so:
n |
old CLI | new shared |
|---|---|---|
| 1 000 | "1k" |
"1.0k" |
| 1 400 | "1k" |
"1.4k" ✓ more precise |
| 1 500 | "2k" |
"1.5k" ✓ more precise |
| 2 000 | "2k" |
"2.0k" |
The precision gain for fractional values is an improvement, but the trailing .0 for exact multiples of 1000 is visual noise in CLI output. The PR notes only list prettifyModel as a user-visible change; this one is undocumented. If intentional, it should be noted in the changelog. If not, parseFloat drops the trailing zero without losing precision:
💡 Proposed fix to strip trailing zeros
- if (n >= 1_000) return `${(n / 1_000).toFixed(1)}k`;
+ if (n >= 1_000) return `${parseFloat((n / 1_000).toFixed(1))}k`;parseFloat("1.0") → 1, parseFloat("1.4") → 1.4, so "1k" / "1.4k" / "1.5k" etc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (n >= 1_000) return `${(n / 1_000).toFixed(1)}k`; | |
| if (n >= 1_000) return `${parseFloat((n / 1_000).toFixed(1))}k`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/format.ts` at line 4, The check that formats thousands
currently returns `${(n / 1_000).toFixed(1)}k`, which produces "1.0k" for exact
thousands; change this to strip trailing ".0" by converting the toFixed string
back to a number before appending "k" (e.g., use parseFloat((n /
1_000).toFixed(1)) or Number(...) to drop the unnecessary ".0"). Update the
expression in the same conditional that contains `if (n >= 1_000) return \`${(n
/ 1_000).toFixed(1)}k\`;` so outputs become "1k", "1.4k", "1.5k", etc., without
altering the fractional precision for non-exact thousands.
Comment on lines
+14
to
+15
| if (/^o4/i.test(normalized)) return "o4"; | ||
| if (/^o3/i.test(normalized)) return "o3"; |
There was a problem hiding this comment.
Don't collapse suffixed o3/o4 identifiers.
These prefix checks rewrite any o3*/o4* model ID to the family name, so labels like o3-mini or o4-mini lose their distinguishing suffix while the GPT branch above deliberately preserves it. That will mislabel distinct models anywhere this shared helper is used.
Suggested fix
- if (/^o4/i.test(normalized)) return "o4";
- if (/^o3/i.test(normalized)) return "o3";
+ if (/^o4$/i.test(normalized)) return "o4";
+ if (/^o3$/i.test(normalized)) return "o3";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (/^o4/i.test(normalized)) return "o4"; | |
| if (/^o3/i.test(normalized)) return "o3"; | |
| if (/^o4$/i.test(normalized)) return "o4"; | |
| if (/^o3$/i.test(normalized)) return "o3"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/models.ts` around lines 14 - 15, The pattern matching on
the normalized model ID incorrectly collapses any string starting with "o3" or
"o4" to the family name; update the checks around the normalized variable so
they only match the bare family or family followed by a hyphen (e.g., use
/^o4(?:$|-)/i and /^o3(?:$|-)/i) to preserve suffixes like "o4-mini" or
"o3-mini" rather than rewriting them to just "o4"/"o3". Locate the two regex
lines that reference normalized and replace them with the stricter
family-or-hyphen matches so suffixes are kept intact.
Adds a `typecheck` task to turbo.json with `dependsOn: ["^build"]` and switches CI from `bun run typecheck` in `apps/web` to `bun run typecheck` at the repo root (which delegates to `turbo typecheck`). Without this, `@straude/shared`'s `dist/` is never built before tsc runs in `apps/web`, producing TS2307 on `@straude/shared/format` and `@straude/shared/models`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Merged into Net: +428 / −1,809 across 70 files. Highlights:
All checks were green at merge (CI, claude-review, CodeRabbit, Vercel). |
This was referenced May 4, 2026
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
1 participant
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.
Summary
Driven by a
slop-scanaudit of the codebase. Five parallel agents on isolated worktrees, integrated locally, one PR.prettifyModel,getShareModelLabel,formatTokenslifted into a new@straude/sharedworkspace, consumed by bothapps/webandpackages/cli(no more silent drift between web and CLI model labels).fillDayscopies → 1; 3 inline focus-trap blocks → 1 hook.getOpenStatsForPagesnapshot read/write failures now emit observable errors instead of silent fallback;users/meinvalid-link rejection preserves the underlying parse error viacause.proxy,getCellColor,buildSubject,buildProfileShareUrl,hasCodexLogsshims gone.stripMarkdownextracted to its own module with 13 unit tests covering each replacement.straude-codemap.html,prometheus-list.png,posthog-setup-report.md), 3 orphaned source files (lib/performance/interaction.ts,landing/GlobalFeed.tsx,cli/lib/codex.ts), 8 strayexportkeywords on module-private helpers, 3 truly-unused symbols.Net: +424 / −1,808 lines across 67 files.
slop-scan delta
structure.duplicate-function-signaturesstructure.pass-through-wrappersdefensive.async-noisedefensive.error-obscuringRemaining findings are intentionally untouched: 6 justified empty catches (idempotent
launchctl/crontab,localStorageaccess in private-mode browsers,readdiron missing Codex sessions dir), 4 Supabase SSRgetAllwrappers (required by@supabase/ssr), 9 directory-fanout-hotspot informational entries, 42tests.duplicate-mock-setup(normal Vitest patterns).Worktree breakdown
tech-debt/inline-pass-throughsstripMarkdownextractiontech-debt/web-internal-deduptech-debt/shared-package@straude/sharedworkspace + cross-package deduptech-debt/error-handlingtech-debt/dead-code-stagetech-debt/cleanupTest plan
bun --cwd apps/web exec tsc --noEmit -p tsconfig.check.json— cleanbun --cwd packages/cli exec tsc --noEmit— cleanbun run lint— clean (0 errors, 0 warnings)apps/webVitest — 558/558 pass (incl. 13 newstripMarkdowncases)packages/cliVitest — 216/216 passbun dev:localsmoke — landing/recap/feed/admin/focus-trap (run before merging)straude push --dry-run— confirms model labels + token counts unchangedbun --cwd apps/web run scripts/generate-og.ts)Risk notes
prettifyModelconsolidation has one user-visible behavior change: legacy Claude IDs (e.g.anthropic/claude-3-opus) now return"Claude Opus"instead of the raw string. The previouslib/utils/post-share.tscopy was missing the.includes()fallback thatActivityCard,open-stats, and the CLI all had — consolidation picked the more complete behavior. Verified by existingprettify-model.test.ts.open-statserror path now emits structured logs (console.errorwith prefixed context + TODOs for PostHog server-side capture). The fallback chain to placeholder stats is preserved.stripMarkdownwas lifted byte-for-byte; an existing ordering quirk (link rule runs before image rule, sobecomes!altnot empty) is locked in by a unit test with a comment. Pre-existing behavior, not changed in this PR.@straude/sharedis a built package (tsc→dist/) consumed via workspace alias. Turbo's^buildchain handles compilation order. Direct.tsconsumption was tried first but broke under CLI's NodeNext resolution.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Removals