job recommendation graph #172
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/registry/app/[username]/jobs-graph/page.js (3)
53-69: Refine error logging in cosineSimilarity function.The error logging is too verbose for a utility function. Consider using a more concise error message or implementing proper error handling.
const cosineSimilarity = (a, b) => { if (!Array.isArray(a) || !Array.isArray(b)) { - console.log('Invalid vectors:', { a, b }); + console.warn('Invalid vectors provided to cosineSimilarity'); return 0; } if (a.length !== b.length) { - console.log('Vector length mismatch:', { - aLength: a.length, - bLength: b.length, - }); + console.warn(`Vector length mismatch: ${a.length} !== ${b.length}`); return 0; } const dotProduct = a.reduce((sum, _, i) => sum + a[i] * b[i], 0); const magnitudeA = Math.sqrt(a.reduce((sum, val) => sum + val * val, 0)); const magnitudeB = Math.sqrt(b.reduce((sum, val) => sum + val * val, 0)); return dotProduct / (magnitudeA * magnitudeB); };🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
77-105: Consider using useReducer for complex state management.The component manages multiple related states (jobs, isLoading, dimensions, activeNode, etc.). Consider using useReducer to simplify state management and improve maintainability.
Example implementation:
const initialState = { jobs: null, isLoading: true, isInitialized: false, dimensions: { width: 0, height: 0 }, activeNode: null, hoveredNode: null, pinnedNode: null, jobInfo: {}, graphData: null, mostRelevant: [], lessRelevant: [], readJobs: new Set(), filterText: '', filteredNodes: new Set(), showSalaryGradient: false, salaryRange: { min: Infinity, max: -Infinity } }; function reducer(state, action) { switch (action.type) { case 'SET_JOBS': return { ...state, jobs: action.payload }; case 'SET_LOADING': return { ...state, isLoading: action.payload }; // ... other cases } }🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
572-729: Simplify graph rendering configuration.The ForceGraph2D component has numerous props and configurations that could be extracted into a separate configuration object for better maintainability.
Consider extracting the graph configuration:
const graphConfig = { nodeLabel: null, nodeColor: getNodeColor, nodeVal: (node) => node.size, linkWidth: (link) => Math.sqrt(link.value) * 2, linkColor: "#cccccc", linkOpacity: 0.3, enableNodeDrag: true, cooldownTicks: 100, warmupTicks: 100, minZoom: 0.1, maxZoom: 5, forceEngine: "d3", d3AlphaDecay: 0.02, d3VelocityDecay: 0.3 }; // Then in JSX: <ForceGraph2D ref={graphRef} graphData={graphData} {...graphConfig} // ... other specific props />🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/registry/app/[username]/jobs-graph/page.js(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/registry/app/[username]/jobs-graph/page.js
[error] 34-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Actions: CI
apps/registry/app/[username]/jobs-graph/page.js
[warning] Code formatting does not meet Prettier standards
🔇 Additional comments (1)
apps/registry/app/[username]/jobs-graph/page.js (1)
71-75: Implement node size calculation logic.The comment indicates missing implementation details for node size calculation. The current implementation uses a default size of 1 with padding.
Consider implementing a more sophisticated size calculation based on node properties such as relevance score, number of connections, or other metrics.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| setIsLoading(true); | ||
| try { | ||
| const response = await axios.post('/api/jobs-graph', { username }); | ||
| const { | ||
| graphData, | ||
| jobInfoMap, | ||
| mostRelevant, | ||
| lessRelevant, | ||
| allJobs, | ||
| resume, | ||
| } = response.data; | ||
|
|
||
| setMostRelevant(mostRelevant); | ||
| setLessRelevant(lessRelevant); | ||
| setJobs(allJobs); | ||
| setJobInfo(jobInfoMap); | ||
| setGraphData(graphData); | ||
| } catch (error) { | ||
| console.error('Error fetching data:', error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| if (username) { | ||
| fetchData(); | ||
| } | ||
| }, [username]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in data fetching.
The error handling in the data fetching effect could be more informative and provide better user feedback.
useEffect(() => {
const fetchData = async () => {
setIsLoading(true);
try {
const response = await axios.post('/api/jobs-graph', { username });
const {
graphData,
jobInfoMap,
mostRelevant,
lessRelevant,
allJobs,
resume,
} = response.data;
setMostRelevant(mostRelevant);
setLessRelevant(lessRelevant);
setJobs(allJobs);
setJobInfo(jobInfoMap);
setGraphData(graphData);
} catch (error) {
- console.error('Error fetching data:', error);
+ const errorMessage = error.response?.data?.message || error.message;
+ console.error('Error fetching jobs data:', errorMessage);
+ // Consider adding state for error handling
+ setError(errorMessage);
} finally {
setIsLoading(false);
}
};
if (username) {
fetchData();
}
}, [username]);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
| const parseSalary = useCallback((salary) => { | ||
| if (!salary) return null; | ||
| if (typeof salary === 'number') return salary; | ||
|
|
||
| const str = salary.toString().toLowerCase(); | ||
| // Extract all numbers from the string | ||
| const numbers = str.match(/\d+(?:\.\d+)?/g); | ||
| if (!numbers) return null; | ||
|
|
||
| // Convert numbers considering k/K multiplier | ||
| const values = numbers.map((num) => { | ||
| const multiplier = str.includes('k') ? 1000 : 1; | ||
| return parseFloat(num) * multiplier; | ||
| }); | ||
|
|
||
| // If range, return average | ||
| if (values.length > 1) { | ||
| values.sort((a, b) => a - b); | ||
| return (values[0] + values[values.length - 1]) / 2; | ||
| } | ||
|
|
||
| return values[0]; | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance salary parsing robustness.
The salary parsing function could handle more edge cases and provide better type safety.
const parseSalary = useCallback((salary) => {
if (!salary) return null;
if (typeof salary === 'number') return salary;
const str = salary.toString().toLowerCase();
- // Extract all numbers from the string
- const numbers = str.match(/\d+(?:\.\d+)?/g);
+ // Extract numbers and handle various formats (k, K, thousand, million)
+ const numbers = str.match(/\d+(?:\.\d+)?(?:k|K|thousand|million)?/g);
if (!numbers) return null;
- // Convert numbers considering k/K multiplier
const values = numbers.map((num) => {
- const multiplier = str.includes('k') ? 1000 : 1;
+ const multiplier =
+ /k|K|thousand/.test(num) ? 1000 :
+ /million/.test(num) ? 1000000 : 1;
+ num = num.replace(/[^0-9.]/g, '');
return parseFloat(num) * multiplier;
});📝 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.
| const parseSalary = useCallback((salary) => { | |
| if (!salary) return null; | |
| if (typeof salary === 'number') return salary; | |
| const str = salary.toString().toLowerCase(); | |
| // Extract all numbers from the string | |
| const numbers = str.match(/\d+(?:\.\d+)?/g); | |
| if (!numbers) return null; | |
| // Convert numbers considering k/K multiplier | |
| const values = numbers.map((num) => { | |
| const multiplier = str.includes('k') ? 1000 : 1; | |
| return parseFloat(num) * multiplier; | |
| }); | |
| // If range, return average | |
| if (values.length > 1) { | |
| values.sort((a, b) => a - b); | |
| return (values[0] + values[values.length - 1]) / 2; | |
| } | |
| return values[0]; | |
| }, []); | |
| const parseSalary = useCallback((salary) => { | |
| if (!salary) return null; | |
| if (typeof salary === 'number') return salary; | |
| const str = salary.toString().toLowerCase(); | |
| // Extract numbers and handle various formats (k, K, thousand, million) | |
| const numbers = str.match(/\d+(?:\.\d+)?(?:k|K|thousand|million)?/g); | |
| if (!numbers) return null; | |
| const values = numbers.map((num) => { | |
| const multiplier = | |
| /k|K|thousand/.test(num) ? 1000 : | |
| /million/.test(num) ? 1000000 : 1; | |
| num = num.replace(/[^0-9.]/g, ''); | |
| return parseFloat(num) * multiplier; | |
| }); | |
| // If range, return average | |
| if (values.length > 1) { | |
| values.sort((a, b) => a - b); | |
| return (values[0] + values[values.length - 1]) / 2; | |
| } | |
| return values[0]; | |
| }, []); |
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
| {activeNode && ( | ||
| <div | ||
| style={{ | ||
| position: 'absolute', | ||
| top: '20px', | ||
| right: '20px', | ||
| maxWidth: '300px', | ||
| backgroundColor: 'rgba(255, 255, 255, 0.95)', | ||
| padding: '10px', | ||
| borderRadius: '5px', | ||
| border: '1px solid #000', | ||
| zIndex: 1000, | ||
| whiteSpace: 'pre-wrap', | ||
| }} | ||
| > | ||
| <div style={{ position: 'relative' }}> | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| justifyContent: 'space-between', | ||
| marginBottom: '10px', | ||
| }} | ||
| > | ||
| <button | ||
| onClick={() => markJobAsRead(activeNode.id)} | ||
| style={{ | ||
| padding: '4px 8px', | ||
| borderRadius: '4px', | ||
| border: '1px solid #ccc', | ||
| backgroundColor: readJobs.has(activeNode.id) | ||
| ? '#e2e8f0' | ||
| : '#fff', | ||
| cursor: 'pointer', | ||
| fontSize: '12px', | ||
| }} | ||
| > | ||
| {readJobs.has(activeNode.id) ? 'Read ✓' : 'Mark as Read'} | ||
| </button> | ||
| <button | ||
| onClick={() => { | ||
| setPinnedNode(null); | ||
| setHoveredNode(null); | ||
| setActiveNode(null); | ||
| }} | ||
| style={{ | ||
| width: '20px', | ||
| height: '20px', | ||
| borderRadius: '50%', | ||
| border: '1px solid #000', | ||
| backgroundColor: 'white', | ||
| cursor: 'pointer', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| fontSize: '12px', | ||
| padding: 0, | ||
| lineHeight: 1, | ||
| }} | ||
| > | ||
| × | ||
| </button> | ||
| </div> | ||
| {formatTooltip(jobInfo[activeNode.id])} | ||
| <div className="mt-2 flex items-center justify-between gap-2"> | ||
| <div className="flex-1"> | ||
| <p className="text-sm text-gray-500"> | ||
| {jobInfo[activeNode.id]?.location?.city || 'Remote'}{' '} | ||
| {jobInfo[activeNode.id]?.type || ''} | ||
| </p> | ||
| </div> | ||
| <a | ||
| href={`/jobs/${activeNode.id}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| View Details | ||
| </a> | ||
| </div> | ||
| {jobInfo[activeNode.id]?.salary && ( | ||
| <p className="text-sm text-gray-500 mt-1"> | ||
| Salary: {jobInfo[activeNode.id].salary} | ||
| </p> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility features to the job details panel.
The job details panel lacks proper accessibility attributes, making it difficult for screen readers to interpret.
{activeNode && (
<div
style={{
position: 'absolute',
top: '20px',
right: '20px',
maxWidth: '300px',
backgroundColor: 'rgba(255, 255, 255, 0.95)',
padding: '10px',
borderRadius: '5px',
border: '1px solid #000',
zIndex: 1000,
whiteSpace: 'pre-wrap',
}}
+ role="dialog"
+ aria-label="Job Details"
+ aria-modal="true"
>📝 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.
| {activeNode && ( | |
| <div | |
| style={{ | |
| position: 'absolute', | |
| top: '20px', | |
| right: '20px', | |
| maxWidth: '300px', | |
| backgroundColor: 'rgba(255, 255, 255, 0.95)', | |
| padding: '10px', | |
| borderRadius: '5px', | |
| border: '1px solid #000', | |
| zIndex: 1000, | |
| whiteSpace: 'pre-wrap', | |
| }} | |
| > | |
| <div style={{ position: 'relative' }}> | |
| <div | |
| style={{ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| marginBottom: '10px', | |
| }} | |
| > | |
| <button | |
| onClick={() => markJobAsRead(activeNode.id)} | |
| style={{ | |
| padding: '4px 8px', | |
| borderRadius: '4px', | |
| border: '1px solid #ccc', | |
| backgroundColor: readJobs.has(activeNode.id) | |
| ? '#e2e8f0' | |
| : '#fff', | |
| cursor: 'pointer', | |
| fontSize: '12px', | |
| }} | |
| > | |
| {readJobs.has(activeNode.id) ? 'Read ✓' : 'Mark as Read'} | |
| </button> | |
| <button | |
| onClick={() => { | |
| setPinnedNode(null); | |
| setHoveredNode(null); | |
| setActiveNode(null); | |
| }} | |
| style={{ | |
| width: '20px', | |
| height: '20px', | |
| borderRadius: '50%', | |
| border: '1px solid #000', | |
| backgroundColor: 'white', | |
| cursor: 'pointer', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| fontSize: '12px', | |
| padding: 0, | |
| lineHeight: 1, | |
| }} | |
| > | |
| × | |
| </button> | |
| </div> | |
| {formatTooltip(jobInfo[activeNode.id])} | |
| <div className="mt-2 flex items-center justify-between gap-2"> | |
| <div className="flex-1"> | |
| <p className="text-sm text-gray-500"> | |
| {jobInfo[activeNode.id]?.location?.city || 'Remote'}{' '} | |
| {jobInfo[activeNode.id]?.type || ''} | |
| </p> | |
| </div> | |
| <a | |
| href={`/jobs/${activeNode.id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| View Details | |
| </a> | |
| </div> | |
| {jobInfo[activeNode.id]?.salary && ( | |
| <p className="text-sm text-gray-500 mt-1"> | |
| Salary: {jobInfo[activeNode.id].salary} | |
| </p> | |
| )} | |
| </div> | |
| </div> | |
| )} | |
| {activeNode && ( | |
| <div | |
| style={{ | |
| position: 'absolute', | |
| top: '20px', | |
| right: '20px', | |
| maxWidth: '300px', | |
| backgroundColor: 'rgba(255, 255, 255, 0.95)', | |
| padding: '10px', | |
| borderRadius: '5px', | |
| border: '1px solid #000', | |
| zIndex: 1000, | |
| whiteSpace: 'pre-wrap', | |
| }} | |
| role="dialog" | |
| aria-label="Job Details" | |
| aria-modal="true" | |
| > | |
| <div style={{ position: 'relative' }}> | |
| <div | |
| style={{ | |
| display: 'flex', | |
| justifyContent: 'space-between', | |
| marginBottom: '10px', | |
| }} | |
| > | |
| <button | |
| onClick={() => markJobAsRead(activeNode.id)} | |
| style={{ | |
| padding: '4px 8px', | |
| borderRadius: '4px', | |
| border: '1px solid #ccc', | |
| backgroundColor: readJobs.has(activeNode.id) | |
| ? '#e2e8f0' | |
| : '#fff', | |
| cursor: 'pointer', | |
| fontSize: '12px', | |
| }} | |
| > | |
| {readJobs.has(activeNode.id) ? 'Read ✓' : 'Mark as Read'} | |
| </button> | |
| <button | |
| onClick={() => { | |
| setPinnedNode(null); | |
| setHoveredNode(null); | |
| setActiveNode(null); | |
| }} | |
| style={{ | |
| width: '20px', | |
| height: '20px', | |
| borderRadius: '50%', | |
| border: '1px solid #000', | |
| backgroundColor: 'white', | |
| cursor: 'pointer', | |
| display: 'flex', | |
| alignItems: 'center', | |
| justifyContent: 'center', | |
| fontSize: '12px', | |
| padding: 0, | |
| lineHeight: 1, | |
| }} | |
| > | |
| × | |
| </button> | |
| </div> | |
| {formatTooltip(jobInfo[activeNode.id])} | |
| <div className="mt-2 flex items-center justify-between gap-2"> | |
| <div className="flex-1"> | |
| <p className="text-sm text-gray-500"> | |
| {jobInfo[activeNode.id]?.location?.city || 'Remote'}{' '} | |
| {jobInfo[activeNode.id]?.type || ''} | |
| </p> | |
| </div> | |
| <a | |
| href={`/jobs/${activeNode.id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| View Details | |
| </a> | |
| </div> | |
| {jobInfo[activeNode.id]?.salary && ( | |
| <p className="text-sm text-gray-500 mt-1"> | |
| Salary: {jobInfo[activeNode.id].salary} | |
| </p> | |
| )} | |
| </div> | |
| </div> | |
| )} |
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting does not meet Prettier standards
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
apps/registry/pages/api/jobs.js (4)
33-33: Consider reducing the temperature value for more consistent outputs.While the prompt improvements are good, the temperature value of 0.85 might lead to inconsistent professional summaries. A lower temperature (e.g., 0.3-0.5) would provide more consistent and focused outputs while maintaining enough creativity for natural language generation.
80-85: Improve handling of missing similarity scores.Using a default similarity score of 0 when no match is found could be misleading, as it doesn't distinguish between "no match found" and "completely different content". Consider using
nullor a special value to indicate missing matches.- similarity: doc ? doc.similarity : 0, + similarity: doc?.similarity ?? null, + matchFound: Boolean(doc),
95-98: Avoid logging sensitive information.The current logging includes job IDs which might be sensitive information. Consider logging only the count of invalid dates or masking the IDs.
- console.log('Invalid date found:', { - id: job.id, - created_at: job.created_at, - }); + console.log('Invalid date found:', { + created_at: job.created_at, + // Mask the ID or omit it entirely + id: `${job.id.substring(0, 4)}...`, + });
122-125: Improve job filtering configuration and readability.The current implementation uses a magic number for the date filter. Consider making this configurable and improving readability.
+ const JOBS_MAX_AGE_DAYS = 60; + const MS_PER_DAY = 24 * 60 * 60 * 1000; + const filteredJobs = jobsWithSimilarity.filter( (job) => - new Date(job.created_at) > - new Date(Date.now() - 60 * 24 * 60 * 60 * 1000), + new Date(job.created_at) > + new Date(Date.now() - JOBS_MAX_AGE_DAYS * MS_PER_DAY), );apps/registry/app/[username]/jobs-graph/page.js (2)
9-12: Add input validation to formatSkills function.The function should validate that
skillsis an array when it's not null.const formatSkills = (skills) => { if (!skills) return ''; + if (!Array.isArray(skills)) return ''; return skills.map((skill) => `${skill.name} (${skill.level})`).join(', '); };
58-85: Consider using useReducer for complex state management.The component manages multiple related states (jobs, loading, dimensions, etc.). Consider using
useReducerto centralize state management and make it more maintainable.+const jobsReducer = (state, action) => { + switch (action.type) { + case 'SET_LOADING': + return { ...state, isLoading: action.payload }; + case 'SET_JOBS_DATA': + return { + ...state, + jobs: action.payload.allJobs, + mostRelevant: action.payload.mostRelevant, + lessRelevant: action.payload.lessRelevant, + jobInfo: action.payload.jobInfoMap, + graphData: action.payload.graphData, + }; + // Add other cases + default: + return state; + } +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/registry/app/[username]/jobs-graph/page.js(1 hunks)apps/registry/pages/api/jobs.js(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/registry/app/[username]/jobs-graph/page.js
[error] 34-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/registry/pages/api/jobs.js
[error] 94-94: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Meticulous Tests
- GitHub Check: build
🔇 Additional comments (5)
apps/registry/pages/api/jobs.js (2)
94-94: UseNumber.isNaNInstead ofisNaNfor Safer Type CheckingThe use of
isNaNis unsafe because it attempts type coercion, which can lead to unexpected results.- if (isNaN(date.getTime())) { + if (Number.isNaN(date.getTime())) {🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
57-64: Review the embedding and matching parameters for potential issues.Several concerns with the current implementation:
- Setting
match_thresholdto -1 effectively disables similarity filtering, potentially returning irrelevant matches.- Increasing
match_countto 500 could impact API performance and response times.- Zero-padding embeddings might affect similarity calculations - consider using dimensionality reduction techniques instead.
Let's verify the impact of these changes:
apps/registry/app/[username]/jobs-graph/page.js (3)
86-108: 🛠️ Refactor suggestionEnhance salary parsing robustness.
The salary parsing function could handle more edge cases and provide better type safety.
const parseSalary = useCallback((salary) => { if (!salary) return null; if (typeof salary === 'number') return salary; const str = salary.toString().toLowerCase(); - // Extract all numbers from the string - const numbers = str.match(/\d+(?:\.\d+)?/g); + // Extract numbers and handle various formats (k, K, thousand, million) + const numbers = str.match(/\d+(?:\.\d+)?(?:k|K|thousand|million)?/g); if (!numbers) return null; - // Convert numbers considering k/K multiplier const values = numbers.map((num) => { - const multiplier = str.includes('k') ? 1000 : 1; + const multiplier = + /k|K|thousand/.test(num) ? 1000 : + /million/.test(num) ? 1000000 : 1; + num = num.replace(/[^0-9.]/g, ''); return parseFloat(num) * multiplier; });Likely invalid or redundant comment.
354-377: 🛠️ Refactor suggestionImprove error handling in data fetching.
The error handling in the data fetching effect could be more informative and provide better user feedback.
+ const [error, setError] = useState(null); useEffect(() => { const fetchData = async () => { setIsLoading(true); + setError(null); try { const response = await axios.post('/api/jobs-graph', { username }); const { graphData, jobInfoMap, mostRelevant, lessRelevant, allJobs } = response.data; setMostRelevant(mostRelevant); setLessRelevant(lessRelevant); setJobs(allJobs); setJobInfo(jobInfoMap); setGraphData(graphData); } catch (error) { - console.error('Error fetching data:', error); + const errorMessage = error.response?.data?.message || error.message; + console.error('Error fetching jobs data:', errorMessage); + setError(errorMessage); } finally { setIsLoading(false); } };Likely invalid or redundant comment.
445-530: 🛠️ Refactor suggestionAdd accessibility features to the job details panel.
The job details panel lacks proper accessibility attributes, making it difficult for screen readers to interpret.
{activeNode && ( <div + role="dialog" + aria-label="Job Details" + aria-modal="true" style={{ position: 'absolute', top: '20px', right: '20px', maxWidth: '300px', backgroundColor: 'rgba(255, 255, 255, 0.95)', padding: '10px', borderRadius: '5px', border: '1px solid #000', zIndex: 1000, whiteSpace: 'pre-wrap', }} >Likely invalid or redundant comment.
| const { data: sortedJobs } = await supabase | ||
| .from('jobs') | ||
| .select('*') | ||
| .in('id', jobIds) | ||
| .order('created_at', { ascending: false }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement pagination for better performance.
The current implementation could face performance issues:
- Using
INclause with up to 500 IDs might impact query performance. - Fetching all jobs at once without pagination could lead to large response payloads and increased memory usage.
Consider implementing cursor-based pagination using Supabase's built-in pagination support.
const { data: sortedJobs } = await supabase
.from('jobs')
.select('*')
.in('id', jobIds)
- .order('created_at', { ascending: false });
+ .order('created_at', { ascending: false })
+ .range(0, 49); // Start with 50 results per page📝 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.
| const { data: sortedJobs } = await supabase | |
| .from('jobs') | |
| .select('*') | |
| .in('id', jobIds) | |
| .order('created_at', { ascending: false }); | |
| const { data: sortedJobs } = await supabase | |
| .from('jobs') | |
| .select('*') | |
| .in('id', jobIds) | |
| .order('created_at', { ascending: false }) | |
| .range(0, 49); // Start with 50 results per page | |
| <a | ||
| href={`/jobs/${activeNode.id}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| View Details | ||
| </a> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security attributes to external link.
The external link needs additional security attributes to prevent potential security vulnerabilities.
<a
href={`/jobs/${activeNode.id}`}
target="_blank"
- rel="noopener noreferrer"
+ rel="noopener noreferrer nofollow"
className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors"
onClick={(e) => e.stopPropagation()}
>
View Details
</a>📝 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.
| <a | |
| href={`/jobs/${activeNode.id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| View Details | |
| </a> | |
| <a | |
| href={`/jobs/${activeNode.id}`} | |
| target="_blank" | |
| rel="noopener noreferrer nofollow" | |
| className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" | |
| onClick={(e) => e.stopPropagation()} | |
| > | |
| View Details | |
| </a> |
| const handleEngineStop = useCallback(() => { | ||
| console.log('ENGINE STOPPED Graph instance:', graphRef.current); | ||
| if (graphRef.current) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug console.log statements.
Debug console.log statements should not be present in production code.
const handleEngineStop = useCallback(() => {
- console.log('ENGINE STOPPED Graph instance:', graphRef.current);
if (graphRef.current) {
if (!isInitialized) {
const fg = graphRef.current;
- console.log('FG IS STARTING', fg);📝 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.
| const handleEngineStop = useCallback(() => { | |
| console.log('ENGINE STOPPED Graph instance:', graphRef.current); | |
| if (graphRef.current) { | |
| const handleEngineStop = useCallback(() => { | |
| if (graphRef.current) { |
| const calculateCollisionRadius = (node) => { | ||
| // Replace this with your logic to determine node size | ||
| const nodeSize = node.size || 1; // Default size if not specified | ||
| return nodeSize + 3; // Add padding if desired | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace placeholder comment with specific node size calculation logic.
The comment indicates that the node size logic needs to be replaced with actual implementation.
const calculateCollisionRadius = (node) => {
- // Replace this with your logic to determine node size
- const nodeSize = node.size || 1; // Default size if not specified
+ // Calculate node size based on relevance and type
+ const baseSize = node.group === -1 ? 80 : (node.size || 1);
+ const padding = node.group === -1 ? 5 : 3;
return nodeSize + padding;
};Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
apps/registry/pages/api/jobs.js (1)
94-94:⚠️ Potential issueUse
Number.isNaNfor safer type checking.Replace
isNaNwithNumber.isNaNto prevent type coercion issues.- if (isNaN(date.getTime())) { + if (Number.isNaN(date.getTime())) {🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
apps/registry/pages/api/jobs-graph.js (2)
26-31:⚠️ Potential issueAdd Error Handling for Missing Resume Data
When fetching the resume data, there's no check to ensure that
data[0]exists before attempting to parsedata[0].resume. If no resume is found for the given username, this will cause a runtime error.Apply this diff:
-const { data } = await supabase +const { data, error } = await supabase .from('resumes') .select() .eq('username', username); +if (error) { + return res.status(500).json({ error: 'Failed to fetch resume data' }); +} +if (!data || data.length === 0) { + return res.status(404).json({ error: 'Resume not found' }); +}
173-174:⚠️ Potential issueAdd Error Handling When Parsing Job Content
Parsing
job.gpt_contentwithout error handling can lead to runtime errors ifgpt_contentis invalid JSON.Apply this diff:
-jobInfoMap[job.uuid] = JSON.parse(job.gpt_content); +try { + jobInfoMap[job.uuid] = JSON.parse(job.gpt_content); +} catch (e) { + console.error(`Failed to parse gpt_content for job ID ${job.uuid}:`, e); + jobInfoMap[job.uuid] = {}; +}apps/registry/app/[username]/jobs-graph/page.js (4)
284-286: 🛠️ Refactor suggestionRemove debug console.log statements.
Debug console.log statements should not be present in production code.
Apply this diff:
const handleEngineStop = useCallback(() => { - console.log('ENGINE STOPPED Graph instance:', graphRef.current); if (graphRef.current) {
356-379: 🛠️ Refactor suggestionImprove error handling in data fetching.
The error handling in the data fetching effect could be more informative and provide better user feedback.
Apply this diff:
useEffect(() => { const fetchData = async () => { setIsLoading(true); + setError(null); try { const response = await axios.post('/api/jobs-graph', { username }); const { graphData, jobInfoMap, mostRelevant, lessRelevant, allJobs } = response.data; setMostRelevant(mostRelevant); setLessRelevant(lessRelevant); setJobs(allJobs); setJobInfo(jobInfoMap); setGraphData(graphData); } catch (error) { - console.error('Error fetching data:', error); + const errorMessage = error.response?.data?.message || error.message; + console.error('Error fetching jobs data:', errorMessage); + setError(errorMessage); } finally { setIsLoading(false); } }; if (username) { fetchData(); } }, [username]);
447-532: 🛠️ Refactor suggestionAdd accessibility features to the job details panel.
The job details panel lacks proper accessibility attributes, making it difficult for screen readers to interpret.
Apply this diff:
{activeNode && ( <div + role="dialog" + aria-label="Job Details" + aria-modal="true" style={{ position: 'absolute', top: '20px', right: '20px',
515-523: 🛠️ Refactor suggestionAdd security attributes to external link.
The external link needs additional security attributes to prevent potential security vulnerabilities.
Apply this diff:
<a href={`/jobs/${activeNode.id}`} target="_blank" - rel="noopener noreferrer" + rel="noopener noreferrer nofollow" className="px-3 py-1 text-sm font-medium text-white bg-blue-600 rounded-md hover:bg-blue-700 transition-colors" onClick={(e) => e.stopPropagation()} > View Details </a>
🧹 Nitpick comments (2)
apps/registry/pages/api/jobs.js (2)
67-69: Enhance logging for better observability.Consider adding more detailed logging to help with monitoring and debugging:
- // Log initial match count - console.log('Total matched jobs:', documents.length); + // Log match statistics + console.log('Job matching stats:', { + totalMatches: documents.length, + avgSimilarity: documents.reduce((sum, doc) => sum + doc.similarity, 0) / documents.length, + maxSimilarity: Math.max(...documents.map(doc => doc.similarity)), + minSimilarity: Math.min(...documents.map(doc => doc.similarity)) + });
122-124: Extract time window configuration.The 60-day window is hardcoded. Consider extracting it to a configuration variable for better maintainability.
+ const JOB_LISTING_WINDOW_DAYS = 60; const filteredJobs = jobsWithSimilarity.filter( (job) => - new Date(job.created_at) > new Date(Date.now() - 60 * 24 * 60 * 60 * 1000) + new Date(job.created_at) > new Date(Date.now() - JOB_LISTING_WINDOW_DAYS * 24 * 60 * 60 * 1000) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml(1 hunks)apps/registry/app/[username]/jobs-graph/page.js(1 hunks)apps/registry/app/api/similarity/route.js(2 hunks)apps/registry/pages/api/jobs-graph.js(1 hunks)apps/registry/pages/api/jobs.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/registry/app/api/similarity/route.js
🧰 Additional context used
🪛 Biome (1.9.4)
apps/registry/pages/api/jobs.js
[error] 94-94: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
apps/registry/app/[username]/jobs-graph/page.js
[error] 34-35: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
apps/registry/pages/api/jobs.js (2)
33-33: LGTM! Improved prompt for better resume analysis.The updated prompt will generate more comprehensive professional summaries by explicitly requesting all significant career aspects.
63-64: 🛠️ Refactor suggestionConsider tuning the matching parameters.
Setting
match_thresholdto -1 removes the similarity filter, which could include irrelevant matches. Consider:
- Using a small positive threshold (e.g., 0.01) to maintain quality.
- Implementing pagination as the increased
match_countcould impact performance.- match_threshold: -1, // Choose an appropriate threshold for your data - match_count: 500, // Choose the number of matches + match_threshold: 0.01, // Filter out irrelevant matches + match_count: 50, // Start with a smaller batchLikely invalid or redundant comment.
apps/registry/app/[username]/jobs-graph/page.js (1)
88-110: Enhance salary parsing robustness.The salary parsing function could handle more edge cases and provide better type safety.
Apply this diff:
const parseSalary = useCallback((salary) => { if (!salary) return null; if (typeof salary === 'number') return salary; const str = salary.toString().toLowerCase(); - // Extract all numbers from the string - const numbers = str.match(/\d+(?:\.\d+)?/g); + // Extract numbers and handle various formats (k, K, thousand, million) + const numbers = str.match(/\d+(?:\.\d+)?(?:k|K|thousand|million)?/g); if (!numbers) return null; - // Convert numbers considering k/K multiplier const values = numbers.map((num) => { - const multiplier = str.includes('k') ? 1000 : 1; + const multiplier = + /k|K|thousand/.test(num) ? 1000 : + /million/.test(num) ? 1000000 : 1; + num = num.replace(/[^0-9.]/g, ''); return parseFloat(num) * multiplier; });
.github/workflows/ci.yml
Outdated
| - name: Install dependencies | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y libasound2 libicu-dev libffi-dev libx264-dev liboss4-salsa-asound2 |
There was a problem hiding this comment.
💡 Codebase verification
Remove unnecessary system dependencies
The codebase shows no evidence of requiring these system-level libraries. All media-related code uses standard web technologies:
- CSS media queries
- HTML5 video embeds
- Text content only
Consider removing these dependencies from the CI workflow to improve build time and reduce complexity. If libicu-dev is specifically needed for Node.js features, please document the use case.
🔗 Analysis chain
Verify the necessity of system dependencies
The addition of system-level libraries (libasound2, libicu-dev, libffi-dev, libx264-dev, liboss4-salsa-asound2) seems excessive for a job visualization feature. These audio/video dependencies are concerning as they:
- Increase CI build time and complexity
- May indicate underlying architectural issues if web components require system libraries
- Could impact maintainability
Could you please:
- Clarify why each dependency is necessary
- Consider if there are lighter alternatives
- Document the reasoning in the PR description
Let's verify which components actually require these dependencies:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential usage of native modules or bindings
rg -l 'require\(.*\.node\)' || rg -l 'require\(.*\.so\)'
# Search for references to audio/video processing
rg -i "audio|video|sound|media" apps/
Length of output: 2117
| const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co'; | ||
| const supabaseKey = process.env.SUPABASE_KEY; | ||
| const supabase = createClient(supabaseUrl, supabaseKey); |
There was a problem hiding this comment.
Move Supabase URL to environment variables.
Hardcoding sensitive information like database URLs is a security risk. Move the URL to environment variables for better security and configuration management.
Apply this diff:
-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co';
+const supabaseUrl = process.env.SUPABASE_URL;
const supabaseKey = process.env.SUPABASE_KEY;
const supabase = createClient(supabaseUrl, supabaseKey);📝 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.
| const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co'; | |
| const supabaseKey = process.env.SUPABASE_KEY; | |
| const supabase = createClient(supabaseUrl, supabaseKey); | |
| const supabaseUrl = process.env.SUPABASE_URL; | |
| const supabaseKey = process.env.SUPABASE_KEY; | |
| const supabase = createClient(supabaseUrl, supabaseKey); |
| const desiredLength = 3072; | ||
| let embedding = completion.data[0].embedding; | ||
| if (embedding.length < desiredLength) { | ||
| embedding = embedding.concat( | ||
| Array(desiredLength - embedding.length).fill(0) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle case where embedding length exceeds desired length.
The code only handles the case where the embedding is shorter than the desired length. Add handling for the case where the embedding is longer to prevent potential issues.
Apply this diff:
const desiredLength = 3072;
let embedding = completion.data[0].embedding;
+if (embedding.length > desiredLength) {
+ embedding = embedding.slice(0, desiredLength);
+} else
if (embedding.length < desiredLength) {
embedding = embedding.concat(
Array(desiredLength - embedding.length).fill(0)
);
}📝 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.
| const desiredLength = 3072; | |
| let embedding = completion.data[0].embedding; | |
| if (embedding.length < desiredLength) { | |
| embedding = embedding.concat( | |
| Array(desiredLength - embedding.length).fill(0) | |
| ); | |
| } | |
| const desiredLength = 3072; | |
| let embedding = completion.data[0].embedding; | |
| if (embedding.length > desiredLength) { | |
| embedding = embedding.slice(0, desiredLength); | |
| } else if (embedding.length < desiredLength) { | |
| embedding = embedding.concat( | |
| Array(desiredLength - embedding.length).fill(0) | |
| ); | |
| } |
Summary by CodeRabbit
Release Notes
New Features
Improvements
Infrastructure