Skip to content

Conversation

@capJavert
Copy link
Contributor

@capJavert capJavert commented Jan 14, 2026

Changes

Enhance anon flow to parse opportunity in the background while user registers. API dailydotdev/daily-api#3443

Events

Did you introduce any new tracking events?

Experiment

Did you introduce any new experiments?

Manual Testing

Caution

Please make sure existing components are not breaking/affected by this PR

Preview domain

https://async-opportunity-parsing.preview.app.daily.dev

@capJavert capJavert self-assigned this Jan 14, 2026
@capJavert capJavert requested a review from a team as a code owner January 14, 2026 16:25
@vercel
Copy link

vercel bot commented Jan 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
daily-webapp Ready Ready Preview Jan 15, 2026 0:18am
1 Skipped Deployment
Project Deployment Review Updated (UTC)
storybook Ignored Ignored Jan 15, 2026 0:18am

@capJavert
Copy link
Contributor Author

@claude review

@claude
Copy link
Contributor

claude bot commented Jan 14, 2026

Claude finished @capJavert's task in 1m 22s —— View job


Code Review

Review Progress:

  • Read all changed files
  • Analyze async opportunity parsing implementation
  • Check for bugs, security issues, and edge cases
  • Review code quality and best practices
  • Provide feedback and recommendations

Summary

This PR implements async opportunity parsing to improve the anon flow by parsing opportunities in the background while users register. Overall, the implementation is solid with good separation of concerns. I've identified several issues that need attention:

Critical Issues

1. Early return prevents form submission - RecruiterJobLinkModal.tsx:110

if (jobLink) {
  const trimmedLink = jobLink.trim();
  if (trimmedLink && validateJobLink(trimmedLink)) {
    payload.url = trimmedLink;
  }

  return;  // ⚠️ PROBLEM: Returns early, never calls parseOpportunity or onSubmit
}

Impact: When a user enters a job link, the form validates it but never actually submits. The function returns early without calling parseOpportunity() or triggering the onSubmit callback.

Fix needed: Remove the early return and restructure the logic:

if (jobLink) {
  const trimmedLink = jobLink.trim();
  if (trimmedLink && !validateJobLink(trimmedLink)) {
    return; // Only return if validation fails
  }
  payload.url = trimmedLink;
} else if (file) {
  payload.file = file;
}

Fix this →

2. Timeout not cancelled on unmount - OpportunityPreviewContext.tsx:46-74

The refetchInterval callback can continue executing after component unmount, potentially causing memory leaks or state updates on unmounted components.

Fix needed: Track the query status and cancel ongoing operations on cleanup.

Major Issues

3. Missing error handling for setPendingOpportunityId - RecruiterJobLinkModal.tsx:118

const opportunity = await parseOpportunity(payload);
await setPendingOpportunityId(opportunity.id);  // ⚠️ No error handling

If setPendingOpportunityId fails (IndexedDB quota exceeded, etc.), the error is silently swallowed and the flow continues, which could lead to lost state.

Recommendation: Add error handling:

try {
  const opportunity = await parseOpportunity(payload);
  await setPendingOpportunityId(opportunity.id);
  // ... continue flow
} catch (error) {
  // Handle persistence errors gracefully
  console.error('Failed to save opportunity state:', error);
  // Still proceed with onSubmit even if persistence fails
}

4. Race condition in analyze.tsx - Lines 42-95

The useNewOpportunityParser hook and OpportunityPreviewContext could race:

  • useNewOpportunityParser calls parseOpportunity for new submissions
  • Meanwhile, OpportunityPreviewContext polls for opportunity state
  • If the mutation completes and navigates before the context updates, stale data might be displayed

Recommendation: Consider coordinating these operations or adding explicit state synchronization.

5. Type mismatch in parseOpportunityMutationOptions - mutations.ts:476

const result = await gqlClient.request<{
  parseOpportunity: Opportunity;
}>(PARSE_OPPORTUNITY_MUTATION, {
  payload: {
    file,
    url,
  },
});

return result.parseOpportunity;

The function returns Opportunity type, but based on the context usage in RecruiterJobLinkModal.tsx:117-118, it appears the API might return an opportunity in PARSING state. However, there's no explicit handling for this in the modal.

Question: Does the API return the opportunity immediately with state: PARSING, or does it block until parsing completes? The implementation seems to assume immediate return, but this should be clarified in the mutation or type definitions.

Code Quality Issues

6. Inconsistent error messages - analyze.tsx:112-114

displayToast(
  'Failed to process your job description. Please try again with a different file or URL.',
);

This error message is hardcoded, while RecruiterJobLinkModal uses getParseOpportunityMutationErrorMessage() for detailed error handling. Consider using the same error message utility for consistency.

7. Magic numbers - OpportunityPreviewContext.tsx:28,61

const parseOpportunityIntervalMs = 3000;
// ...
const maxRetries = (oneMinute * 1000) / parseOpportunityIntervalMs;

Consider extracting these to a shared constants file for better maintainability:

export const PARSING_CONFIG = {
  POLL_INTERVAL_MS: 3000,
  MAX_PARSING_TIME_MS: 60000, // 1 minute
  MAX_PREVIEW_TIME_MS: 120000, // 2 minutes
} as const;

8. Unused import - index.tsx:20

import { webappUrl } from '@dailydotdev/shared/src/lib/constants';

This import appears to be used only on line 46, but it's redundant since router.push already handles relative paths. Consider using relative paths consistently.

Minor Issues

9. Comment clarity - analyze.tsx:88-90

// Consider parsing in progress if:
// 1. The mutation is currently pending, OR
// 2. We're on a new submission and parsing hasn't completed yet

While helpful, this comment could be clearer about why we need condition #2. Suggest:

// Consider parsing in progress if:
// 1. The mutation is currently pending (actively calling parseOpportunity), OR
// 2. We're on a new submission that hasn't finished parsing yet
//    (mutation succeeded but parseOpportunity API call may still be processing on backend)

10. Missing JSDoc - usePersistentContext.ts:27-32

The hook is well-structured, but the return tuple would benefit from JSDoc explaining what each position represents:

/**
 * @returns Tuple of [value, updateValue, isFetched, isLoading]
 */
export default function usePersistentContext<T>(...): UserPersistentContextType<T>

Positive Aspects

Good separation of concerns: The parsing logic is well-separated between mutation, context, and UI layers
Proper polling implementation: The refetchInterval logic correctly handles max retries and state transitions
Type safety: Good use of TypeScript types throughout
User experience: The async parsing provides a better UX by not blocking on registration
Error boundaries: Good use of error states and user feedback via toasts
Persistent state: Using IndexedDB to preserve parsing state across page reloads is smart

Testing Recommendations

Consider testing these scenarios:

  1. User submits URL, closes browser during parsing, returns later
  2. IndexedDB quota exceeded during setPendingOpportunityId
  3. Parsing times out after 1 minute
  4. User submits file instead of URL (ensure both paths work)
  5. Race condition: user navigates away mid-parse

Summary

Must fix before merge:

Should fix before merge:

Consider for follow-up:


@capJavert capJavert force-pushed the async-opportunity-parsing branch from 7a8c99b to eb1ce2b Compare January 15, 2026 12:15
@capJavert capJavert merged commit aff5ec7 into main Jan 15, 2026
11 checks passed
@capJavert capJavert deleted the async-opportunity-parsing branch January 15, 2026 12:20
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.

2 participants