fix: replace explicit any usage with real types - #372
Conversation
Fixes all 17 hand-written any occurrences found across the codebase (apps/api and apps/web; packages/ui was already clean): - schema.ts: the Drizzle forward-referenced FK callback now returns AnySQLiteColumn (Drizzle's own documented type for exactly this circular/forward-reference pattern) instead of any. - ReactPdfDocumentRenderer.tsx: textStyle now typed as Style from @react-pdf/types (added as a proper devDependency instead of relying on it as an undeclared transitive dependency of @react-pdf/renderer). - setup.ts: the localStorage polyfill needed no cast at all — globalThis.localStorage is already ambiently typed as Storage. - The 6 optimistic-update setQueryData/getQueryData callsites (ApplicationDetailPage, ContactsTab, DocumentsTab x2, InterviewsTab, ApplicationsPage) now pass the same typed generic every other setQueryData call in each of those files already used, instead of leaving this one instance untyped. - useDebouncedCallback.ts's generic constraint any[] is kept — a documented, legitimate TypeScript pattern for generic callback-wrapping utilities (unknown[] breaks bivariant inference for specific-parameter callbacks) — but the per-line disable comment is replaced with @typescript-eslint/no-explicit-any's own ignoreRestArgs option in eslint.config.mjs, so the exception is a real config decision instead of a silent one-off suppression. routeTree.gen.ts (TanStack Router's generated file, already eslint-disabled/@ts-nocheck) is untouched — not hand-written code.
|
Warning Review limit reached
Next review available in: 49 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe pull request replaces selected ChangesType safety and optimistic cache updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx`:
- Around line 306-319: Update the onDelete optimistic deletion flow to use the
complete query key produced by applicationsPageQueryOptions, including status,
starred, likelyGhosted, and searchTerm, for its getQueryData, setQueryData, and
rollback/restore operations. Ensure deletion and undo target the currently
visible filtered applications query rather than only ['applications', 'page'].
In
`@apps/web/src/routes/_authenticated/applications/`$applicationId/-components/ApplicationDetailPage.tsx:
- Around line 484-493: Add tests covering optimistic deletion and undo
restoration for all six handlers, using a controllable showUndoToast mock:
ApplicationDetailPage.tsx lines 484-493, ContactsTab.tsx lines 350-359,
DocumentsTab.tsx lines 242-251 and 396-405, InterviewsTab.tsx lines 425-436, and
ApplicationsPage.tsx lines 306-319. Verify document-draft deletion, optimistic
cache removal, restoration on undo, and removal of the deleted item across every
ApplicationsPage pagination page; retain existing mutation-request assertions.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e4f3d27-cf0a-499f-a943-a8409f43a2ee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
apps/api/package.jsonapps/api/src/infrastructure/db/schema.tsapps/api/src/infrastructure/pdf/ReactPdfDocumentRenderer.tsxapps/web/eslint.config.mjsapps/web/src/__tests__/setup.tsapps/web/src/hooks/useDebouncedCallback.tsapps/web/src/routes/_authenticated/applications/$applicationId/-components/ApplicationDetailPage.tsxapps/web/src/routes/_authenticated/applications/$applicationId/-components/ContactsTab.tsxapps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsxapps/web/src/routes/_authenticated/applications/$applicationId/-components/InterviewsTab.tsxapps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx
💤 Files with no reviewable changes (1)
- apps/web/src/hooks/useDebouncedCallback.ts
| const snapshot = qc.getQueryData<{ notes: Note[] }>([ | ||
| 'notes', | ||
| applicationId, | ||
| ]); | ||
| qc.setQueryData<{ notes: Note[] }>( | ||
| ['notes', applicationId], | ||
| (prev) => ({ | ||
| notes: (prev?.notes ?? []).filter((n) => n.id !== note.id), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '=== diff stat ==='
git diff --stat
printf '%s\n' '=== changed files ==='
git diff --name-only
printf '%s\n' '=== candidate test files ==='
git ls-files | rg '(^|/)(__tests__/|.*\.(test|spec)\.(ts|tsx)$)' | rg 'ApplicationDetailPage|ContactsTab|DocumentsTab|InterviewsTab|ApplicationsPage|applications'
printf '%s\n' '=== relevant symbols and test references ==='
rg -n --glob '*.ts' --glob '*.tsx' \
'ApplicationDetailPage|ContactsTab|DocumentsTab|InterviewsTab|ApplicationsPage|optimistic|undo|restore|setQueryData|getQueryData' \
apps/web/src/routes/_authenticated/applicationsRepository: mankatcheung/job-finder
Length of output: 15552
🏁 Script executed:
#!/bin/bash
set -eu
for f in \
apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx \
apps/web/src/__tests__/components/ApplicationsPage.test.tsx \
apps/web/src/__tests__/components/ContactsTab.test.tsx \
apps/web/src/__tests__/components/DocumentsTab.test.tsx \
apps/web/src/__tests__/components/InterviewsTab.test.tsx
do
printf '\n=== %s ===\n' "$f"
wc -l "$f"
ast-grep outline "$f" || true
printf '%s\n' '--- targeted references ---'
rg -n -C 5 \
'delete|remove|undo|showUndoToast|onUndo|notes|contacts|documentDrafts|documents|interviewRounds|applications|infinite|page' \
"$f" || true
doneRepository: mankatcheung/job-finder
Length of output: 36398
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '=== ApplicationDetailPage handler and test ==='
sed -n '470,505p' \
apps/web/src/routes/_authenticated/applications/\$applicationId/-components/ApplicationDetailPage.tsx
sed -n '475,510p' \
apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx
printf '%s\n' '=== ContactsTab handler and test ==='
sed -n '340,370p' \
apps/web/src/routes/_authenticated/applications/\$applicationId/-components/ContactsTab.tsx
sed -n '120,151p' \
apps/web/src/__tests__/components/ContactsTab.test.tsx
printf '%s\n' '=== DocumentsTab handlers and test ==='
sed -n '232,265p;386,418p' \
apps/web/src/routes/_authenticated/applications/\$applicationId/-components/DocumentsTab.tsx
sed -n '108,138p' \
apps/web/src/__tests__/components/DocumentsTab.test.tsx
printf '%s\n' '=== InterviewsTab handler and test ==='
sed -n '415,450p' \
apps/web/src/routes/_authenticated/applications/\$applicationId/-components/InterviewsTab.tsx
sed -n '100,128p' \
apps/web/src/__tests__/components/InterviewsTab.test.tsx
printf '%s\n' '=== ApplicationsPage handler and tests ==='
sed -n '295,340p' \
apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx
sed -n '430,485p' \
apps/web/src/__tests__/components/ApplicationsPage.test.tsxRepository: mankatcheung/job-finder
Length of output: 16323
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
sites = [
(
"ApplicationDetailPage",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/ApplicationDetailPage.tsx"),
Path("apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx"),
"notes",
"deletes a note",
),
(
"ContactsTab",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/ContactsTab.tsx"),
Path("apps/web/src/__tests__/components/ContactsTab.test.tsx"),
"contacts",
"deletes a contact",
),
(
"DocumentsTab draft",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx"),
Path("apps/web/src/__tests__/components/DocumentsTab.test.tsx"),
"documentDrafts",
"deletes a draft",
),
(
"DocumentsTab document",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx"),
Path("apps/web/src/__tests__/components/DocumentsTab.test.tsx"),
"documents",
"deletes a document",
),
(
"InterviewsTab",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/InterviewsTab.tsx"),
Path("apps/web/src/__tests__/components/InterviewsTab.test.tsx"),
"interviewRounds",
"deletes an interview round",
),
(
"ApplicationsPage",
Path("apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx"),
Path("apps/web/src/__tests__/components/ApplicationsPage.test.tsx"),
"applications",
"bulk-deletes selected applications",
),
]
for name, source_path, test_path, cache_key, test_name in sites:
source = source_path.read_text()
tests = test_path.read_text()
implementation_has_undo = (
f"setQueryData(['{cache_key}'" in source
and "onUndo:" in source
)
match = re.search(
rf"\bit\(['\"]{re.escape(test_name)}['\"].*?(?=\n\s*\}\);|\n\s*\}}\);)",
tests,
re.S,
)
block = match.group(0) if match else ""
has_cache_assertion = any(
token in block
for token in ("getQueryData", "setQueryData", "getQueryData", "getQueryCache", "queryClient.get")
)
has_undo_assertion = "onUndo" in block or "undo" in block.lower() and "expect" in block
print(
f"{name}: implementation_onUndo={implementation_has_undo}; "
f"delete_test={'present' if match else 'missing'}; "
f"cache_assertion={has_cache_assertion}; undo_assertion={has_undo_assertion}"
)
mock = Path("apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx").read_text()
print(
"shared_mock: "
f"executes_onExecute={'onExecute();' in mock}; "
f"invokes_onUndo={'onUndo' in mock.split('vi.mock(\\'`#/lib/undoToast`\\'')[1].split('}));', 1)[0]}"
)
PYRepository: mankatcheung/job-finder
Length of output: 390
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
sites = [
(
"ApplicationDetailPage",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/ApplicationDetailPage.tsx"),
Path("apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx"),
"notes",
"deletes a note",
),
(
"ContactsTab",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/ContactsTab.tsx"),
Path("apps/web/src/__tests__/components/ContactsTab.test.tsx"),
"contacts",
"deletes a contact",
),
(
"DocumentsTab draft",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx"),
Path("apps/web/src/__tests__/components/DocumentsTab.test.tsx"),
"documentDrafts",
"deletes a draft",
),
(
"DocumentsTab document",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx"),
Path("apps/web/src/__tests__/components/DocumentsTab.test.tsx"),
"documents",
"deletes a document",
),
(
"InterviewsTab",
Path("apps/web/src/routes/_authenticated/applications/$applicationId/-components/InterviewsTab.tsx"),
Path("apps/web/src/__tests__/components/InterviewsTab.test.tsx"),
"interviewRounds",
"deletes an interview round",
),
(
"ApplicationsPage",
Path("apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx"),
Path("apps/web/src/__tests__/components/ApplicationsPage.test.tsx"),
"applications",
"bulk-deletes selected applications",
),
]
for name, source_path, test_path, cache_key, test_name in sites:
source = source_path.read_text()
tests = test_path.read_text()
implementation_has_undo = (
f"setQueryData(['{cache_key}'" in source
and "onUndo:" in source
)
match = re.search(
rf"\bit\(['\"]{re.escape(test_name)}['\"].*?(?=\n\s*\}\);|\n\s*\}}\);)",
tests,
re.S,
)
block = match.group(0) if match else ""
has_cache_assertion = any(
token in block
for token in ("getQueryData", "setQueryData", "getQueryCache", "queryClient.get")
)
has_undo_assertion = "onUndo" in block or (
"undo" in block.lower() and "expect" in block
)
print(
f"{name}: implementation_onUndo={implementation_has_undo}; "
f"delete_test={'present' if match else 'missing'}; "
f"cache_assertion={has_cache_assertion}; undo_assertion={has_undo_assertion}"
)
mock = Path("apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx").read_text()
mock_section = mock.split("vi.mock('`#/lib/undoToast`'", 1)[1].split("}));", 1)[0]
print(
"shared_mock: "
f"executes_onExecute={'onExecute();' in mock_section}; "
f"invokes_onUndo={'onUndo' in mock_section}"
)
PYRepository: mankatcheung/job-finder
Length of output: 390
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
sites = [
(
"ApplicationDetailPage",
"apps/web/src/routes/_authenticated/applications/$applicationId/-components/ApplicationDetailPage.tsx",
"apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx",
"deletes a note",
),
(
"ContactsTab",
"apps/web/src/routes/_authenticated/applications/$applicationId/-components/ContactsTab.tsx",
"apps/web/src/__tests__/components/ContactsTab.test.tsx",
"deletes a contact",
),
(
"DocumentsTab draft",
"apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx",
"apps/web/src/__tests__/components/DocumentsTab.test.tsx",
"deletes a draft",
),
(
"DocumentsTab document",
"apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx",
"apps/web/src/__tests__/components/DocumentsTab.test.tsx",
"deletes a document",
),
(
"InterviewsTab",
"apps/web/src/routes/_authenticated/applications/$applicationId/-components/InterviewsTab.tsx",
"apps/web/src/__tests__/components/InterviewsTab.test.tsx",
"deletes an interview round",
),
(
"ApplicationsPage",
"apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx",
"apps/web/src/__tests__/components/ApplicationsPage.test.tsx",
"bulk-deletes selected applications",
),
]
for name, source_name, test_name_path, test_name in sites:
source = Path(source_name).read_text()
tests = Path(test_name_path).read_text()
pattern = (
r"\bit\(['\"]" + re.escape(test_name)
+ r"['\"].*?(?=\n\s*\}\);)"
)
match = re.search(pattern, tests, re.S)
block = match.group(0) if match else ""
print(
f"{name}: "
f"implementation_onUndo={'onUndo:' in source}; "
f"delete_test={'present' if match else 'missing'}; "
f"cache_assertion={any(x in block for x in ('getQueryData', 'setQueryData', 'getQueryCache'))}; "
f"undo_assertion={'onUndo' in block or ('undo' in block.lower() and 'expect' in block)}"
)
mock = Path("apps/web/src/__tests__/components/ApplicationDetailPage.test.tsx").read_text()
mock_section = mock.split("vi.mock('`#/lib/undoToast`'", 1)[1].split("}));", 1)[0]
print(
"shared_mock: "
f"executes_onExecute={'onExecute();' in mock_section}; "
f"invokes_onUndo={'onUndo' in mock_section}"
)
PYRepository: mankatcheung/job-finder
Length of output: 890
Add coverage for optimistic deletion and undo restoration.
The existing deletion tests assert mutation requests only. No test covers document-draft deletion, optimistic cache removal, or undo restoration. Use a controllable showUndoToast mock and cover all six handlers, including removal across all ApplicationsPage pages.
📍 Affects 5 files
apps/web/src/routes/_authenticated/applications/$applicationId/-components/ApplicationDetailPage.tsx#L484-L493(this comment)apps/web/src/routes/_authenticated/applications/$applicationId/-components/ContactsTab.tsx#L350-L359apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx#L242-L251apps/web/src/routes/_authenticated/applications/$applicationId/-components/DocumentsTab.tsx#L396-L405apps/web/src/routes/_authenticated/applications/$applicationId/-components/InterviewsTab.tsx#L425-L436apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx#L306-L319
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/src/routes/_authenticated/applications/`$applicationId/-components/ApplicationDetailPage.tsx
around lines 484 - 493, Add tests covering optimistic deletion and undo
restoration for all six handlers, using a controllable showUndoToast mock:
ApplicationDetailPage.tsx lines 484-493, ContactsTab.tsx lines 350-359,
DocumentsTab.tsx lines 242-251 and 396-405, InterviewsTab.tsx lines 425-436, and
ApplicationsPage.tsx lines 306-319. Verify document-draft deletion, optimistic
cache removal, restoration on undo, and removal of the deleted item across every
ApplicationsPage pagination page; retain existing mutation-request assertions.
Source: Coding guidelines
CodeRabbit review on #372 flagged that the bulk onDelete handler's optimistic update used an incomplete query key (['applications', 'page']) instead of the full key applicationsPageQueryOptions actually registers the query under (status, starred, likelyGhosted, searchTerm included). getQueryData/setQueryData require an exact key match — so the optimistic removal and its undo-restore were silently no-ops whenever any filter was active, leaving the deleted rows visible until the mutation's eventual invalidation refetch caught up. Extracts applicationsPageQueryKey() as the single source of truth for this key (used by both applicationsPageQueryOptions and the delete handler), and threads status/starred/likelyGhosted/searchTerm down to BulkActionBar so it can build the matching key. Adds a regression test verifying the optimistic removal specifically: confirmed it fails against the pre-fix code (Stripe/Vercel stay visible) and passes with the fix. Not adding equivalent tests for the other 5 optimistic-update handlers CodeRabbit's second comment named (ApplicationDetailPage, ContactsTab, DocumentsTab x2, InterviewsTab) — verified each already uses the exact same simple query key its own useQuery call uses, so there's no matching bug there; that suggestion is general coverage improvement unrelated to a defect, out of scope for this fix.
|
Addressed CodeRabbit's review (commit 68166f9): Finding 1 (real bug, fixed): the Finding 2 (test coverage suggestion): reviewed each of the other 5 handlers named ( |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/__tests__/components/ApplicationsPage.test.tsx (1)
459-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise a non-default filter in this regression test.
The test renders the default route state, so it only exercises the empty-filter cache key. It does not verify that
status,starred,likelyGhosted, andsearchTermremain aligned when a filter is active. Configure at least one non-default filter before deletion and keep the assertion that rows disappear before the refetch returns.As per coding guidelines, every new or changed utility function must ship with matching tests in the same change. This also matches the PR objective to cover filtered optimistic deletion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/__tests__/components/ApplicationsPage.test.tsx` around lines 459 - 481, Update the “optimistically removes deleted applications from the list” test to render or configure a non-default filter before selecting and deleting applications, exercising the aligned status, starred, likelyGhosted, and searchTerm cache-key values. Preserve the assertions that deleted rows disappear before refetch and that the delete mutation is requested; add or update same-change tests for any new or modified utility used to configure or handle the filtered state.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/routes/_authenticated/applications/index.tsx`:
- Around line 89-103: Update applicationsPageQueryKey so undefined starred and
likelyGhosted values map to null rather than false, preserving explicit false
values as false. Keep the query key aligned with applicationsPageQueryOptions to
ensure absent filters and false filters produce distinct keys, including for
optimistic updates in ApplicationsPage.
---
Nitpick comments:
In `@apps/web/src/__tests__/components/ApplicationsPage.test.tsx`:
- Around line 459-481: Update the “optimistically removes deleted applications
from the list” test to render or configure a non-default filter before selecting
and deleting applications, exercising the aligned status, starred,
likelyGhosted, and searchTerm cache-key values. Preserve the assertions that
deleted rows disappear before refetch and that the delete mutation is requested;
add or update same-change tests for any new or modified utility used to
configure or handle the filtered state.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 74bb0d96-b2b9-4d6e-a4d9-be55ea223471
📒 Files selected for processing (3)
apps/web/src/__tests__/components/ApplicationsPage.test.tsxapps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsxapps/web/src/routes/_authenticated/applications/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx
| export function applicationsPageQueryKey( | ||
| status: string | undefined, | ||
| starred: boolean | undefined, | ||
| searchTerm: string, | ||
| likelyGhosted: boolean | undefined, | ||
| ) { | ||
| return [ | ||
| 'applications', | ||
| 'page', | ||
| status ?? null, | ||
| starred ?? false, | ||
| likelyGhosted ?? false, | ||
| searchTerm, | ||
| ] as const; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep an absent filter distinct from false.
Lines [98-100] map undefined to false, but applicationsPageQueryOptions sends null for an absent starred filter and preserves an explicit false. It also omits an absent likelyGhosted value but sends false when supplied. These requests can therefore share one TanStack Query key while representing different filters.
apps/web/src/routes/_authenticated/applications/-components/ApplicationsPage.tsx Line [326] uses this key for optimistic updates, so the collision can display or update the wrong filtered list. Use a canonical absence value that is distinct from false, such as null, for both key segments.
Proposed fix
- starred ?? false,
- likelyGhosted ?? false,
+ starred ?? null,
+ likelyGhosted ?? null,📝 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.
| export function applicationsPageQueryKey( | |
| status: string | undefined, | |
| starred: boolean | undefined, | |
| searchTerm: string, | |
| likelyGhosted: boolean | undefined, | |
| ) { | |
| return [ | |
| 'applications', | |
| 'page', | |
| status ?? null, | |
| starred ?? false, | |
| likelyGhosted ?? false, | |
| searchTerm, | |
| ] as const; | |
| } | |
| export function applicationsPageQueryKey( | |
| status: string | undefined, | |
| starred: boolean | undefined, | |
| searchTerm: string, | |
| likelyGhosted: boolean | undefined, | |
| ) { | |
| return [ | |
| 'applications', | |
| 'page', | |
| status ?? null, | |
| starred ?? null, | |
| likelyGhosted ?? null, | |
| searchTerm, | |
| ] as const; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/_authenticated/applications/index.tsx` around lines 89 -
103, Update applicationsPageQueryKey so undefined starred and likelyGhosted
values map to null rather than false, preserving explicit false values as false.
Keep the query key aligned with applicationsPageQueryOptions to ensure absent
filters and false filters produce distinct keys, including for optimistic
updates in ApplicationsPage.
…gression test CodeRabbit nitpick on #372: the existing regression test only rendered the default (no-filter) route state, so it only exercised the empty-filter cache key — not the actual scenario the original bug was about (the optimistic delete missing the currently-visible *filtered* query's cache entry). Adds a second test with status='applied' active, keeping the same disappearing-rows assertion.
|
Fixed the nitpick (commit c493882): added a second regression test with an active filter (`status: 'applied'`), so the delete-optimistic-update test covers the actual filtered-query scenario the original bug was about, not just the empty-filter default state. |
1 participant
Summary
Fixes all 17 hand-written
anyoccurrences in the codebase (apps/api+apps/web;packages/uiwas already clean). Found viapnpm lint's@typescript-eslint/no-explicit-anywarnings (14) plus a direct grep cross-check that surfaced 3 more suppressed via inlineeslint-disablecomments.apps/web/src/routeTree.gen.ts(TanStack Router's generated file, already blanketeslint-disable+@ts-nocheck) is untouched — not hand-written.schema.ts: the Drizzle forward-referenced FK callback (document.sourceDraftId→documentDraft.id, defined later in the file) now returnsAnySQLiteColumn— Drizzle's own documented type for exactly this circular/forward-reference pattern — instead ofany.ReactPdfDocumentRenderer.tsx:textStylenow typed asStylefrom@react-pdf/types, added as a properdevDependencyinstead of relying on it as an undeclared transitive dependency of@react-pdf/renderer(pnpm's strict linking correctly blocked the phantom import).setup.ts: turns out no cast was needed at all —globalThis.localStorageis already ambiently typed asStoragein TS's DOM lib; the mock object structurally satisfies it.setQueryData/getQueryDatacall sites (ApplicationDetailPage,ContactsTab,DocumentsTabx2,InterviewsTab,ApplicationsPage) — each of these files already has the same mutation pattern typed correctly elsewhere; these one-off instances were just missing the same generic.ApplicationsPage's was the trickiest (an infinite-query cache value) — typed asInfiniteData<ApplicationsPageResult>using the app's own already-exported result type.useDebouncedCallback.ts's generic constraintany[]is kept — a documented, legitimate TypeScript pattern for generic callback-wrapping utilities (unknown[]breaks bivariant inference for callbacks with specific parameter types). Replaced the per-line disable comment with@typescript-eslint/no-explicit-any's ownignoreRestArgsoption ineslint.config.mjs, so the exception is a real, permanent config decision instead of a silent one-off suppression.Test plan
pnpm --filter @job-finder/api typecheck/test(209 files, 1275 tests) — passpnpm --filter @job-finder/web typecheck/test(60 files, 407 tests) — passpnpm lint(full workspace) — 0 errors, 0 warnings (previously 14no-explicit-anywarnings)pnpm build— all 5 workspace packages build successfullyany-type patterns — only the intentionaluseDebouncedCallback.tsone remains, now covered by the config option instead of a suppression comment🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor
Chores