Uh oh!
There was an error while loading. Please reload this page.
fix: stabilize resume contact inference parsing - #133
Conversation
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces filename-aware resume text extraction throughout the resume processing pipeline, adding support for multiple document formats (PDF, DOCX, DOC) via new dependencies (pdfminer.six and python-docx). Resume helpers in the CRM cog and shared extractor now accept an optional filename parameter, enabling format-specific parsing, improved name detection by filtering heading-like tokens, and filename-aware profile caching. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/unit/test_resume_extractor.py (1)
395-404: Consider parameterizing heading variants for stronger regression coverage.This test currently validates
Resume:only. Adding cases likeResume :,CV:, andProfile:would harden the guard behavior against formatting variations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_resume_extractor.py` around lines 395 - 404, Update the test_extract_name_skips_resume_heading_lines unit test to parameterize heading variants so it covers formatting differences; use pytest.mark.parametrize to pass headings like "Resume:", "Resume :", "CV:", and "Profile:" into the test, call ResumeProfileExtractor.extract with each heading followed by the same sample body, and assert that result.name == "Jane Doe" for every variant; keep the test function name test_extract_name_skips_resume_heading_lines and reuse the ResumeProfileExtractor.extract invocation and assertion logic.apps/discord_bot/src/five08/discord_bot/cogs/crm.py (1)
3290-3293: DOC file extraction is a best-effort fallback, not proper parsing.The
.docformat is a binary OLE compound document, not plain text. The current approach of UTF-8 decoding with non-printable character stripping may extract some readable fragments, but will miss structured content and potentially produce garbled output.Consider adding a dedicated DOC parser (e.g.,
textract,antiword, orpython-docx2txt) for reliable extraction. Alternatively, document this as a known limitation and consider rejecting.docfiles with a message suggesting.docxor🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py` around lines 3290 - 3293, The current DOC handling (branch checking extension == ".doc" which decodes file_content into extracted_text) treats a binary OLE .doc as plain UTF-8 and yields unreliable output; replace this fallback with a proper parser or explicit rejection: either integrate a DOC extraction library (e.g., textract, antiword, python-docx2txt) to populate extracted_text reliably in the same code path, or reject ".doc" uploads (return an informative message asking for .docx/.pdf) and log the limitation. Update the branch that inspects extension and uses file_content/extracted_text to call the chosen parser or perform the rejection and ensure any error paths log the filename/extension for debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/five08/resume_extractor.py`:
- Around line 593-595: The check that creates lowered uses .rstrip(":") which
can leave trailing spaces (e.g. "resume "), causing membership tests against
_PLACEHOLDER_NAME_TOKENS and _NAME_HEADING_TOKENS to fail; update the
normalization to strip trailing whitespace and colons (e.g. use .rstrip(" :") or
first .rstrip(":").rstrip() or otherwise ensure both spaces and ":" are removed)
for the variable lowered (and the same change at the other occurrence around
lines referencing the same logic) so comparisons against
_PLACEHOLDER_NAME_TOKENS and _NAME_HEADING_TOKENS behave correctly.
- Around line 1245-1247: The regex fragment in resume_extractor.py that uses
\b500\+? and \b1000\+? can over-match prefixes of larger numbers (e.g., "5000");
update those tokens to ensure they match whole numeric tokens only by preventing
adjacent digits—replace occurrences of \b500\+? and \b1000\+? with a pattern
that asserts no digit on either side such as (?<!\d)500\+?(?!\d) and
(?<!\d)1000\+?(?!\d) respectively (modify the same combined pattern string shown
in the diff so the impact_score/seniority logic uses the tightened tokens).
---
Nitpick comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 3290-3293: The current DOC handling (branch checking extension ==
".doc" which decodes file_content into extracted_text) treats a binary OLE .doc
as plain UTF-8 and yields unreliable output; replace this fallback with a proper
parser or explicit rejection: either integrate a DOC extraction library (e.g.,
textract, antiword, python-docx2txt) to populate extracted_text reliably in the
same code path, or reject ".doc" uploads (return an informative message asking
for .docx/.pdf) and log the limitation. Update the branch that inspects
extension and uses file_content/extracted_text to call the chosen parser or
perform the rejection and ensure any error paths log the filename/extension for
debugging.
In `@tests/unit/test_resume_extractor.py`:
- Around line 395-404: Update the test_extract_name_skips_resume_heading_lines
unit test to parameterize heading variants so it covers formatting differences;
use pytest.mark.parametrize to pass headings like "Resume:", "Resume :", "CV:",
and "Profile:" into the test, call ResumeProfileExtractor.extract with each
heading followed by the same sample body, and assert that result.name == "Jane
Doe" for every variant; keep the test function name
test_extract_name_skips_resume_heading_lines and reuse the
ResumeProfileExtractor.extract invocation and assertion logic.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/discord_bot/pyproject.tomlapps/discord_bot/src/five08/discord_bot/cogs/crm.pypackages/shared/src/five08/resume_extractor.pytests/unit/test_crm.pytests/unit/test_resume_extractor.py
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
This PR stabilizes resume contact inference for the Discord CRM bot by extracting text from common resume file formats before parsing, carrying filename context through inference/create-contact flows, and hardening name/seniority heuristics to avoid common parsing failures.
Changes:
- Add filename-aware document text extraction in the Discord CRM cog (PDF/DOCX/TXT, plus best-effort DOC) and thread
filenamethrough inference and contact creation flows. - Harden shared resume extraction: skip heading-style “names” (e.g.,
Resume:) and fix a malformed seniority regex that could throw a regex compilation error. - Add regression tests covering heading-name rejection, filename-aware extraction wiring, and seniority inference behavior; update bot dependencies to include
pdfminer.sixandpython-docx.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
apps/discord_bot/src/five08/discord_bot/cogs/crm.py | Adds filename-aware resume text extraction + name validation and threads filename through CRM inference/create flows. |
packages/shared/src/five08/resume_extractor.py | Rejects heading/placeholder names during normalization/extraction; fixes seniority inference regex. |
tests/unit/test_crm.py | Adds tests ensuring heading-like names fall back and filename-aware extraction is used. |
tests/unit/test_resume_extractor.py | Adds tests for heading-name skipping and seniority regex regression. |
apps/discord_bot/pyproject.toml | Adds required dependencies for PDF/DOCX extraction. |
uv.lock | Locks new dependencies (pdfminer-six, python-docx) and their transitive deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif extension == ".doc": | ||
| extracted_text = file_content.decode("utf-8", errors="ignore") | ||
| extracted_text = re.sub(r"[^\x20-\x7E\n\r\t]", " ", extracted_text) | ||
| extracted_text = re.sub(r"\s+", " ", extracted_text).strip() |
There was a problem hiding this comment.
In the .doc branch, re.sub(r"\s+", " ", ...) collapses newlines into spaces, which makes downstream heuristics that rely on splitlines() (e.g., name fallback scanning the first N lines) ineffective and likely causes .doc uploads to fall back to "Unknown Contact" even when the header has a clear name/email. Consider preserving line breaks (only normalize spaces/tabs and/or collapse runs of blank lines) so the later line-based parsing continues to work for .doc content.
| extracted_text=re.sub(r"\s+", " ", extracted_text).strip() | |
| # Preserve line breaks for downstream line-based heuristics: | |
| # - normalize line endings to '\n' | |
| # - collapse runs of spaces/tabs within lines | |
| # - optionally collapse excessive blank lines | |
| extracted_text=extracted_text.replace("\r\n", "\n").replace("\r", "\n") | |
| extracted_text=re.sub(r"[ \t]+", " ", extracted_text) | |
| extracted_text=re.sub(r"\n{3,}", "\n\n", extracted_text).strip() |
Description
Improves Discord-side resume contact inference by extracting text from PDF/DOC/DOCX/TXT before parsing identity hints, threading filename context through inference and create-contact flows, and rejecting heading placeholders like
Resume:as names.Fixes a malformed seniority regex in shared resume extraction that could raise
missing ), unterminated subpattern at position 2and adds safeguards for heading-style names in shared normalization.Adds regression tests for heading-name rejection, filename-aware text extraction in CRM, and seniority regex behavior, and updates bot dependencies/lockfile to include
pdfminer.sixandpython-docx.Related Issue
None.
How Has This Been Tested?
uv run pytest -q tests/unit/test_resume_extractor.py tests/unit/test_crm.py -k "resume or infer or upload_resume"anduv run ruff check apps/discord_bot/src/five08/discord_bot/cogs/crm.py packages/shared/src/five08/resume_extractor.py tests/unit/test_resume_extractor.py tests/unit/test_crm.py.Summary by CodeRabbit
Release Notes
New Features
Tests