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. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change extends the resume CRM workflow with seniority-level parsing and override functionality. It introduces helper functions to normalize seniority labels and extract seniority from extracted profiles, adds a UI select component for seniority override, and enhances the resume confirmation view to display parsed seniority and apply user-selected overrides to update payloads. Changes
Sequence DiagramsequenceDiagram
participant User as User/Discord
participant Extract as Resume Extraction
participant Parse as _extract_parsed_seniority()
participant View as ResumeUpdateConfirmationView
participant Select as ResumeSeniorityOverrideSelect
participant Updates as proposed_updates
User->>Extract: Submit resume for extraction
Extract-->>Parse: Return extracted_profile
Parse-->>View: Compute parsed_seniority
View->>View: Display embed with Parsed Seniority
View->>View: Initialize seniority_override state
View->>Select: Add select component if parsed_seniority present
User->>Select: Choose seniority override (optional)
Select->>View: Trigger _set_seniority_override()
View->>Updates: Apply override to proposed_updates
View-->>User: Confirm with updated payload
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.
Pull request overview
This PR adds a seniority override feature to the resume update confirmation workflow in the Discord CRM bot. When a resume is parsed and a seniority level is extracted, users now see it in the preview embed and can override it via a new dropdown select menu before confirming CRM updates.
Changes:
- New helper functions
_format_seniority_labeland_extract_parsed_seniorityfor normalizing and extracting seniority data from parsed resume profiles. - New
ResumeSeniorityOverrideSelectDiscord UI component and corresponding wiring inResumeUpdateConfirmationView(including_set_seniority_override) plus a guard against empty apply requests. - Unit tests covering label formatting, seniority extraction, and view wiring.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
apps/discord_bot/src/five08/discord_bot/cogs/crm.py |
Adds seniority helper functions, new ResumeSeniorityOverrideSelect UI select, updates ResumeUpdateConfirmationView with override support, adds parsed seniority to the preview embed, and guards against empty confirm_updates calls. |
tests/unit/test_crm.py |
Adds parametrized tests for _format_seniority_label and _extract_parsed_seniority, plus tests for view wiring and the seniority override setter. |
One issue found:
In _format_seniority_label, lines 86–87 are unreachable dead code:
if normalized in labels:
return labels[normalized]
if normalized == "midlevel": # ← dead code: "midlevel" is already a key in `labels`
normalized = "mid-level"
return normalized.title()Since "midlevel" is already a key in the labels dict above (mapping to "Mid-level"), the return labels[normalized] call on line 85 will always be reached before line 86 when normalized == "midlevel". The block at lines 86–87 can never execute.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if normalized == "midlevel": | ||
| normalized = "mid-level" |
There was a problem hiding this comment.
Lines 86–87 are dead code. normalized == "midlevel" is already covered by the labels dictionary lookup immediately above (line 84–85), which returns "Mid-level" for "midlevel" and exits the function. The reassignment on line 87 can never be reached.
| if normalized == "midlevel": | |
| normalized = "mid-level" |
![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 the current code and only fix it if needed.
Inline comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 768-773: The new ResumeSeniorityOverrideSelect (the
discord.ui.Select added via self.add_item) remains enabled after the
confirm/cancel final actions, allowing stale interactions; update the confirm
and cancel handlers (the methods that currently disable only buttons) to also
disable the ResumeSeniorityOverrideSelect instance before editing the message or
stopping the view—locate the select by its class name
ResumeSeniorityOverrideSelect on the current View, set its disabled property to
True (or call view.remove_item/clear_items as appropriate), then perform the
same interaction.response.edit_message/update that you use to disable buttons so
the dropdown cannot be changed after finalization.
- Around line 693-703: The placeholder constructed in __init__ uses parsed_label
= _format_seniority_label(parsed_seniority) without length checks, which can
make the final placeholder exceed Discord's 150-char limit and break rendering;
fix by truncating or ellipsizing parsed_label to a safe length before building
the placeholder (ensure f"Override seniority (parsed: {parsed_label})" is <=150
chars), then pass the truncated label into super().__init__; update the logic in
the __init__ method where parsed_label is used and keep the truncation
centralized (e.g., a small helper or inline truncation) so all placeholder text
stays within the limit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0ffd168-f4bc-4e5c-aa4e-36fa92eca37a
📒 Files selected for processing (2)
apps/discord_bot/src/five08/discord_bot/cogs/crm.pytests/unit/test_crm.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
apps/discord_bot/src/five08/discord_bot/cogs/crm.py:2482
_extract_parsed_seniorityis called twice on the same underlying data within a single request flow: once at line 2246 inside_build_resume_preview_embed(to add the "Parsed Seniority" embed field) and again at line 2482 in_run_resume_extract_and_preview(to decide whether to show the view and which seniority to pass in). Because_build_resume_preview_embeddoesn't include the parsed seniority in its return value, the caller has to recompute it. This means the embed and the view are powered by two separate calls, creating a subtle maintenance risk: if one call site diverges (e.g., one passes a different value or there's a bug in one path), the displayed embed seniority and the view's dropdown could get out of sync. Consider extending the return type of_build_resume_preview_embedtotuple[discord.Embed, dict[str, Any], str | None]to include the already-computedparsed_seniority, and drop the extra call at line 2482.
if isinstance(extracted_profile, dict):
confidence = extracted_profile.get("confidence")
source = extracted_profile.get("source")
if confidence is not None or source:
embed.add_field(
name="Extraction",
value=f"Source: `{source or 'unknown'}` | Confidence: `{confidence}`",
inline=False,
)
parsed_seniority = _extract_parsed_seniority(extracted_profile)
if parsed_seniority:
embed.add_field(
name="Parsed Seniority",
value=f"`{_format_seniority_label(parsed_seniority)}`",
inline=True,
)
if link_member:
embed.add_field(
name="Discord Link",
value=f"Will link contact to {link_member.mention}",
inline=False,
)
profile_url = f"{self.base_url}/#Contact/view/{contact_id}"
embed.add_field(name="🔗 CRM Profile", value=f"[View in CRM]({profile_url})")
return embed, proposed_updates
def _build_role_suggestions_embed(
self,
*,
contact_name: str,
extracted_profile: dict[str, Any],
current_discord_roles: list[str] | None = None,
) -> discord.Embed | None:
"""Build a separate embed suggesting Discord roles to add based on resume data.
Only ever suggests additions — roles are never removed.
Never suggests roles in DISCORD_ROLES_NEVER_SUGGEST.
"""
skills: list[str] = extracted_profile.get("skills") or []
primary_roles: list[str] = extracted_profile.get("primary_roles") or []
country: str | None = extracted_profile.get("address_country")
technical = suggest_technical_discord_roles(skills, primary_roles)
locality = suggest_locality_discord_roles(country)
# Filter roles that should never be suggested
technical = [r for r in technical if r not in DISCORD_ROLES_NEVER_SUGGEST]
locality = [r for r in locality if r not in DISCORD_ROLES_NEVER_SUGGEST]
# If we know the member's current roles, only show missing ones
if current_discord_roles is not None:
existing = set(current_discord_roles)
technical = [r for r in technical if r not in existing]
locality = [r for r in locality if r not in existing]
if not technical and not locality:
return None
embed = discord.Embed(
title="🏷️ Suggested Discord Roles",
description=f"Roles to **add** for **{contact_name}** based on resume — never remove existing roles.",
color=0x57F287,
)
if technical:
embed.add_field(
name="Technical",
value=" ".join(f"`{r}`" for r in technical),
inline=False,
)
if locality:
embed.add_field(
name="Locality",
value=" ".join(f"`{r}`" for r in locality),
inline=False,
)
return embed
async def _run_resume_extract_and_preview(
self,
interaction: discord.Interaction,
contact_id: str,
contact_name: str,
attachment_id: str,
filename: str,
link_member: discord.Member | None,
*,
action: str = "crm.upload_resume",
status_message: str | None = None,
) -> None:
"""Kick off worker extraction and show confirmation preview."""
action_name = action
status_text = (
status_message or "📥 Resume uploaded. Extracting profile fields now..."
)
try:
job_id = await self._enqueue_resume_extract_job(
contact_id=contact_id,
attachment_id=attachment_id,
filename=filename,
)
except Exception as exc:
logger.error("Failed to enqueue resume extract job: %s", exc)
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"stage": "extract_enqueue",
"error": str(exc),
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
"⚠️ Resume uploaded, but extraction job could not be enqueued.",
ephemeral=True,
)
return
await interaction.followup.send(
status_text,
ephemeral=True,
)
try:
job = await self._wait_for_backend_job_result(job_id)
except Exception as exc:
logger.error("Worker polling failed for job_id=%s error=%s", job_id, exc)
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"job_id": job_id,
"stage": "extract_polling",
"error": str(exc),
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
"⚠️ Resume uploaded, but extraction polling failed.",
ephemeral=True,
)
return
if not job:
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"job_id": job_id,
"stage": "extract_timeout",
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
"⚠️ Timed out waiting for extraction result. Try again in a moment.",
ephemeral=True,
)
return
status = str(job.get("status", "unknown"))
if status != "succeeded":
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"job_id": job_id,
"stage": "extract_failed",
"job_status": status,
"last_error": str(job.get("last_error", "")),
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
f"❌ Extraction job failed (status: {status}). "
f"Error: {job.get('last_error') or 'Unknown error'}",
ephemeral=True,
)
return
result = job.get("result")
if not isinstance(result, dict):
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"job_id": job_id,
"stage": "extract_malformed_result",
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
"❌ Extraction result was empty or malformed.",
ephemeral=True,
)
return
if not result.get("success", False):
self._audit_command(
interaction=interaction,
action=action_name,
result="error",
metadata={
"filename": filename,
"attachment_id": attachment_id,
"job_id": job_id,
"stage": "extract_unsuccessful",
"error": str(result.get("error", "")),
},
resource_type="crm_contact",
resource_id=str(contact_id),
)
await interaction.followup.send(
f"❌ Resume extraction failed: {result.get('error') or 'Unknown error'}",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -699,6 +783,20 @@ def __init__( | |||
| self.contact_name = contact_name | |||
| self.proposed_updates = proposed_updates | |||
| self.link_discord = link_discord | |||
| self.parsed_seniority = parsed_seniority | |||
There was a problem hiding this comment.
The seniority_override instance attribute is set in __init__ and updated in _set_seniority_override, but it is never read anywhere in the class or codebase. The actual effect of the override is applied directly through self.proposed_updates["cSeniority"] = value, making the seniority_override field redundant dead state. It should either be removed (since proposed_updates["cSeniority"] already tracks the override) or actively used — for example, to conditionally skip the confirm_updates guard or for audit logging purposes.
| discord.SelectOption(label="Senior", value="senior"), | ||
| discord.SelectOption(label="Staff", value="staff"), | ||
| ] | ||
| super().__init__( | ||
| placeholder=placeholder, | ||
| min_values=1, | ||
| max_values=1, | ||
| options=options, | ||
| custom_id="resume_seniority_override", | ||
| ) | ||
|
|
||
| async def callback(self, interaction: discord.Interaction) -> None: | ||
| view = self.view | ||
| if not isinstance(view, ResumeUpdateConfirmationView): | ||
| await interaction.response.send_message( | ||
| "❌ Unable to update seniority override.", |
There was a problem hiding this comment.
The ResumeSeniorityOverrideSelect.callback method has no test coverage. In particular, the error-handling branch at lines 721–726 (when self.view is not a ResumeUpdateConfirmationView) and the success path (interaction sends the formatted override message) are untested. Given that this file has comprehensive test coverage for other callbacks and view interactions, covering this callback would be consistent with the existing testing conventions.
Description
Related Issue
How Has This Been Tested?
Summary by CodeRabbit