feat: loose candidate matching with Discord role sync#148
Conversation
- Fixed job requirement extraction to parse concise technical skills only (no soft skills like "self-directed") - Added discord_role_types classification to map job postings to Discord role names - Updated candidate search to match on ANY required skill OR any discord role type (loose matching) - Added update_person_discord_roles() to sync Discord roles without touching CRM fields - Added /sync-discord-roles command to bulk-sync all guild member roles into candidate database - Added on_member_update listener for automatic live role syncing when members' roles change - Updated /match-candidates output to show matched Discord roles and role types Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Discord role classification and sync: new constants and JobRequirements field for discord_role_types, candidate search and ranking use matched_discord_roles, a DB updater persists discord_roles, and the Discord bot exposes a manual sync command plus listeners to perform automatic and startup role synchronization. Changes
Sequence Diagram(s)sequenceDiagram
participant Discord as Discord Server
participant Bot as Discord Bot
participant DB as Database
participant Search as Candidate Search
rect rgba(100,150,200,0.5)
Note over Discord,Bot: Role change triggers automatic sync
Discord->>Bot: on_member_update(member roles changed)
activate Bot
Bot->>DB: update_person_discord_roles(discord_user_id, roles)
activate DB
DB-->>Bot: update result (bool)
deactivate DB
Bot-->>Bot: log result / handle errors
deactivate Bot
end
rect rgba(150,200,100,0.5)
Note over Bot,Search: Matching workflow with discord roles
Bot->>Search: match_candidates(job_requirements with discord_role_types)
activate Search
Search->>DB: query candidates including discord_roles
activate DB
DB-->>Search: candidate rows with discord_roles & role_matched flag
deactivate DB
Search->>Search: compute matched_discord_roles, rank by role_match + skills
Search-->>Bot: CandidateMatch[] (includes matched_discord_roles)
deactivate Search
Bot-->>Discord: display results (role badges)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/discord_bot/src/five08/discord_bot/cogs/crm.py (1)
6279-6291:⚠️ Potential issue | 🟠 MajorAllow role-only matches in
/match-candidates.The current gate rejects requests when
required_skillsis empty, even ifdiscord_role_typesis present. That blocks the new OR-based role matching flow.🔧 Proposed fix
- if not requirements.required_skills: + if not requirements.required_skills and not requirements.discord_role_types: self._audit_command( interaction=interaction, action="crm.match_candidates", result="denied", - metadata={"stage": "no_required_skills_extracted"}, + metadata={"stage": "no_required_skills_or_roles_extracted"}, ) await interaction.followup.send( - "⚠️ No required skills could be extracted from this posting. " - "Please include explicit skill requirements and try again.", + "⚠️ No required skills or role types could be extracted from this posting. " + "Please include explicit technical requirements and try again.", ephemeral=True, ) return🤖 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 6279 - 6291, The check in crm.match_candidates currently rejects when requirements.required_skills is empty, blocking role-only matches; update the conditional so it only denies when both required_skills and discord_role_types are empty (e.g., if not requirements.required_skills and not requirements.discord_role_types), keep the same _audit_command call but adjust metadata stage if desired (e.g., "no_skills_or_roles_extracted"), and leave the interaction.followup.send message as the fallback for truly empty requirements so OR-based role matching can proceed when discord_role_types are present.
🧹 Nitpick comments (1)
apps/discord_bot/src/five08/discord_bot/cogs/crm.py (1)
6470-6475: Align live sync with bulk sync bot filtering.
sync_discord_rolesskips bot accounts, buton_member_updatecurrently does not. Mirroring that check keeps behavior consistent and avoids unnecessary writes.🔧 Proposed fix
async def on_member_update( self, before: discord.Member, after: discord.Member, ) -> None: """Automatically sync discord_roles when a member's roles change.""" + if after.bot: + return if before.roles == after.roles: return🤖 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 6470 - 6475, The on_member_update handler currently proceeds even for bot accounts whereas sync_discord_roles filters them; modify the start of the on_member_update logic (the function named on_member_update) to return early when the updated member (after) is a bot (e.g., if after.bot: return) before comparing roles, so the later logic that builds role_names (using DISCORD_ROLES_EXCLUDE_FROM_SYNC and after.roles) and any subsequent sync/write operations are skipped for bots, matching sync_discord_roles behavior.
🤖 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 6409-6433: Docstring and runtime behavior disagree: the docstring
for the sync routine says the "Member" role is excluded but the code actually
uses the DISCORD_ROLES_EXCLUDE_FROM_SYNC constant (in the loop over member.roles
inside the sync command) which currently does not contain "Member". Fix by
making the source of truth consistent — either add "Member" to
DISCORD_ROLES_EXCLUDE_FROM_SYNC or update the docstring to list the exact roles
in DISCORD_ROLES_EXCLUDE_FROM_SYNC; locate the constant
DISCORD_ROLES_EXCLUDE_FROM_SYNC and the sync function (the loop over
member.roles) and modify the constant or the docstring so they match.
- Around line 6425-6445: The bulk sync loop calling update_person_discord_roles
via asyncio.to_thread should not let a single exception abort the entire run;
wrap the await asyncio.to_thread(...) call inside a try/except around the
invocation in the for member in members loop, catch Exception as e, log the
error including the member id and role_names, increment a failure/ skipped
counter (e.g., skipped or add a new failed counter) on exception, and continue
to the next member so other members are processed; keep existing behavior for
did_update when no exception is raised.
In `@packages/shared/src/five08/job_match.py`:
- Around line 234-240: discord_role_types filtering is brittle because it relies
on exact matches; normalize both the whitelist and incoming labels (e.g., strip
whitespace and lowercase) before comparing. Update construction of
valid_role_names from DISCORD_SKILL_ROLE_NAMES to a normalized set (e.g.,
{s.strip().lower() for s in DISCORD_SKILL_ROLE_NAMES}) and map values from
_coerce_str_list(data.get("discord_role_types")) through the same normalization
(strip().lower()) before checking membership to populate discord_role_types.
---
Outside diff comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 6279-6291: The check in crm.match_candidates currently rejects
when requirements.required_skills is empty, blocking role-only matches; update
the conditional so it only denies when both required_skills and
discord_role_types are empty (e.g., if not requirements.required_skills and not
requirements.discord_role_types), keep the same _audit_command call but adjust
metadata stage if desired (e.g., "no_skills_or_roles_extracted"), and leave the
interaction.followup.send message as the fallback for truly empty requirements
so OR-based role matching can proceed when discord_role_types are present.
---
Nitpick comments:
In `@apps/discord_bot/src/five08/discord_bot/cogs/crm.py`:
- Around line 6470-6475: The on_member_update handler currently proceeds even
for bot accounts whereas sync_discord_roles filters them; modify the start of
the on_member_update logic (the function named on_member_update) to return early
when the updated member (after) is a bot (e.g., if after.bot: return) before
comparing roles, so the later logic that builds role_names (using
DISCORD_ROLES_EXCLUDE_FROM_SYNC and after.roles) and any subsequent sync/write
operations are skipped for bots, matching sync_discord_roles behavior.
ℹ️ Review info
Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69dac7c1-9e3f-47f6-b4c8-133c0670c1c5
📒 Files selected for processing (4)
apps/discord_bot/src/five08/discord_bot/cogs/crm.pypackages/shared/src/five08/audit.pypackages/shared/src/five08/candidate_search.pypackages/shared/src/five08/job_match.py
… use Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…nges Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Normalize discord_role_types by casefold+strip before whitelist filtering to tolerate case/whitespace drift from the LLM response - Make _bulk_sync_guild_roles resilient: per-member failures are caught, logged, and counted rather than aborting the whole sync run - Fix docstring to accurately list excluded roles (Bots, FixTweet, @everyone) - Add normalization/dedup tests for discord_role_types extraction Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/unit/test_candidate_search.py (1)
123-134: Missingdiscord_role_typesdefault in_make_requirementshelper.The helper does not include a default for
discord_role_types, butJobRequirementshas a default factory for it. While this works because the dataclass provides a default, adding it explicitly would improve clarity and consistency with how it's used in tests like line 267.♻️ Suggested improvement
def _make_requirements(**overrides: object) -> JobRequirements: defaults = dict( required_skills=["python"], preferred_skills=[], + discord_role_types=[], seniority=None, location_type=None, preferred_timezones=[], raw_location_text=None, title=None, ) defaults.update(overrides) return JobRequirements(**defaults) # type: ignore[arg-type]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/test_candidate_search.py` around lines 123 - 134, The _make_requirements helper omits the discord_role_types default while JobRequirements expects/has one; update the defaults dict inside _make_requirements to include a discord_role_types key (e.g., an empty list) so tests are explicit and consistent with usages like in tests at line ~267; modify the defaults in the _make_requirements function and keep the rest of the merging/return using JobRequirements(**defaults) unchanged.
🤖 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 6495-6516: The on_member_update listener currently processes bot
accounts; update it to early-return for bots to match bulk-sync behavior: in the
on_member_update method of the Cog, add a check (e.g., if after.bot: return)
before computing role_names and calling update_person_discord_roles so bot
members are skipped and no update_person_discord_roles(settings, str(after.id),
role_names) is attempted for bot users.
- Around line 6477-6487: The audit write call to self._audit_command in the
sync-discord-roles flow must be made best-effort so a failing audit never
prevents sending the completion summary; wrap the self._audit_command(...)
invocation in a try/except (catch broad Exception), log or record the error
(e.g., via self.logger.warning or similar) and swallow it so the handler
continues, referencing the sync-discord-roles command handler and the
_audit_command call to locate the change.
- Around line 6438-6455: The current on_ready listener calls
_bulk_sync_guild_roles on every reconnect; add a one-time guard to prevent
repeated runs by storing and checking a boolean flag (e.g., self._roles_synced)
on the Cog: in on_ready, return immediately if self._roles_synced is True,
otherwise set it True and run the sync; alternatively, move the startup sync
into setup_hook and spawn a background task (asyncio.create_task) that calls
_bulk_sync_guild_roles for each guild to ensure the sync runs only once at
startup.
---
Nitpick comments:
In `@tests/unit/test_candidate_search.py`:
- Around line 123-134: The _make_requirements helper omits the
discord_role_types default while JobRequirements expects/has one; update the
defaults dict inside _make_requirements to include a discord_role_types key
(e.g., an empty list) so tests are explicit and consistent with usages like in
tests at line ~267; modify the defaults in the _make_requirements function and
keep the rest of the merging/return using JobRequirements(**defaults) unchanged.
ℹ️ Review info
Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c17e5bc8-a57e-4efd-b72a-dbe4fd848c54
📒 Files selected for processing (4)
apps/discord_bot/src/five08/discord_bot/cogs/crm.pypackages/shared/src/five08/job_match.pytests/unit/test_candidate_search.pytests/unit/test_job_match.py
| @commands.Cog.listener() | ||
| async def on_ready(self) -> None: | ||
| """Bulk-sync all guild member roles on startup.""" | ||
| for guild in self.bot.guilds: | ||
| try: | ||
| updated, skipped, failed = await self._bulk_sync_guild_roles(guild) | ||
| logger.info( | ||
| "Startup discord role sync: guild=%s updated=%d skipped=%d failed=%d", | ||
| guild.name, | ||
| updated, | ||
| skipped, | ||
| failed, | ||
| ) | ||
| except Exception as exc: | ||
| logger.warning( | ||
| "Startup discord role sync failed for guild %s: %s", guild.name, exc | ||
| ) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
discord.py on_ready listener reconnect behavior 2.6.0
💡 Result:
In discord.py 2.6.0, on_ready is not a “run once” startup hook—it can fire again during the bot’s lifetime.
- Initial start:
on_readyfires after the library finishes processing Discord’s READY state (after a fresh IDENTIFY). Discord’s Gateway sends READY after Identify. [2] - Reconnects: discord.py will try to RESUME after a disconnect; if the resume succeeds, you should expect
on_resumed(not a secondon_ready) because Discord finishes a successful replay with a RESUMED event. [2] - Resume fails → new session: if RESUME fails and the library must re-IDENTIFY, discord.py can call
on_readyagain (this is explicitly warned in the docs and commonly observed). [1][3]
Practical implication: don’t start “one-time” background tasks / startup logic directly in on_ready without a guard, because it may run multiple times. [1]
Sources: [1] discord.py v2.6.0 FAQ (warning that on_ready() may be called many times) • [2] Discord Gateway docs (READY after Identify; RESUMED after Resume) • [3] StackOverflow quoting discord.py docs (“on_ready … called whenever a RESUME request fails”).
🌐 Web query:
discord.py recommended pattern run code once on startup bot ready event
💡 Result:
on_ready is not guaranteed to run only once (it can fire again on reconnect/resume failures), so it’s not a reliable place for “run once at startup” logic. Use setup_hook() for one-time startup setup instead. [1], [2]
Recommended patterns
A) Pure “startup” code (doesn’t need the gateway cache): use setup_hook()
from discord.ext import commands
class MyBot(commands.Bot):
async def setup_hook(self):
# runs exactly once, before any events are dispatched
# good for: loading extensions, syncing app commands, creating tasks, etc.
await self.load_extension("cogs.admin")
# await self.tree.sync() # if you need it
bot = MyBot(command_prefix="!")
bot.run(TOKEN)setup_hook() is called once during login() and before events, and is explicitly recommended over on_ready() for setup. [1]
B) “Run once when the bot is actually ready” (needs guild/channel cache): start a task, then wait_until_ready()
import asyncio
from discord.ext import commands
class MyBot(commands.Bot):
async def setup_hook(self):
self.loop.create_task(self.after_ready_once())
async def after_ready_once(self):
await self.wait_until_ready() # safe here; don't do this inside setup_hook directly
# code here runs once, after the bot becomes ready
print("ready-only init ran once")
bot = MyBot(command_prefix="!")
bot.run(TOKEN)For recurring background loops, the official tasks.loop(...).before_loop pattern is to await bot.wait_until_ready() before the first iteration. [3]
Key warning: don’t wait_until_ready() inside setup_hook() itself (it can deadlock because the websocket isn’t connected yet). [1]
Sources:
[1] discord.py API reference — setup_hook() semantics and warning about deadlock
[2] discord.py FAQ — on_ready may be called many times
[3] discord.py discord.ext.tasks docs — before_loop + wait_until_ready() pattern
Prevent startup bulk sync from re-running on every reconnect.
on_ready is not a "run once" hook—it fires again when the bot reconnects and RESUME fails, triggering a full re-IDENTIFY. This causes repeated guild syncs and avoidable load spikes.
The guard pattern will work, but discord.py 2.6.0 recommends using setup_hook() with a background task:
💡 Recommended approach (cleaner)
- `@commands.Cog.listener`()
- async def on_ready(self) -> None:
- """Bulk-sync all guild member roles once per process startup."""
- if getattr(self, "_startup_role_sync_done", False):
- return
- self._startup_role_sync_done = True
- for guild in self.bot.guilds:
+ async def cog_load(self) -> None:
+ """Initialize startup sync task."""
+ self.bot.loop.create_task(self._startup_sync_once())
+
+ async def _startup_sync_once(self) -> None:
+ """Bulk-sync all guild member roles exactly once after bot ready."""
+ await self.bot.wait_until_ready()
+ for guild in self.bot.guilds:Or use the simpler guard if you prefer to keep the listener pattern (both prevent re-runs).
🤖 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 6438 -
6455, The current on_ready listener calls _bulk_sync_guild_roles on every
reconnect; add a one-time guard to prevent repeated runs by storing and checking
a boolean flag (e.g., self._roles_synced) on the Cog: in on_ready, return
immediately if self._roles_synced is True, otherwise set it True and run the
sync; alternatively, move the startup sync into setup_hook and spawn a
background task (asyncio.create_task) that calls _bulk_sync_guild_roles for each
guild to ensure the sync runs only once at startup.
| self._audit_command( | ||
| interaction=interaction, | ||
| action="crm.sync_discord_roles", | ||
| result="success", | ||
| metadata={ | ||
| "updated": updated, | ||
| "skipped_no_db_match": skipped, | ||
| "failed": failed, | ||
| "total_members_scanned": updated + skipped + failed, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Guard audit write in /sync-discord-roles so command success response is never blocked.
If _audit_command(...) raises, this flow can fail before sending the completion summary.
💡 Proposed hardening
- self._audit_command(
- interaction=interaction,
- action="crm.sync_discord_roles",
- result="success",
- metadata={
- "updated": updated,
- "skipped_no_db_match": skipped,
- "failed": failed,
- "total_members_scanned": updated + skipped + failed,
- },
- )
+ try:
+ self._audit_command(
+ interaction=interaction,
+ action="crm.sync_discord_roles",
+ result="success",
+ metadata={
+ "updated": updated,
+ "skipped_no_db_match": skipped,
+ "failed": failed,
+ "total_members_scanned": updated + skipped + failed,
+ },
+ )
+ except Exception as exc:
+ logger.warning("sync_discord_roles: audit write failed: %s", exc)As per coding guidelines: "Audit logging must be best effort: command flows in Discord cogs should never fail if audit writes fail".
🤖 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 6477 -
6487, The audit write call to self._audit_command in the sync-discord-roles flow
must be made best-effort so a failing audit never prevents sending the
completion summary; wrap the self._audit_command(...) invocation in a try/except
(catch broad Exception), log or record the error (e.g., via self.logger.warning
or similar) and swallow it so the handler continues, referencing the
sync-discord-roles command handler and the _audit_command call to locate the
change.
| @commands.Cog.listener() | ||
| async def on_member_update( | ||
| self, | ||
| before: discord.Member, | ||
| after: discord.Member, | ||
| ) -> None: | ||
| """Automatically sync discord_roles when a member's roles change.""" | ||
| if before.roles == after.roles: | ||
| return | ||
|
|
||
| role_names = [ | ||
| r.name for r in after.roles if r.name not in DISCORD_ROLES_EXCLUDE_FROM_SYNC | ||
| ] | ||
|
|
||
| try: | ||
| await asyncio.to_thread( | ||
| update_person_discord_roles, | ||
| settings, | ||
| str(after.id), | ||
| role_names, | ||
| ) | ||
| except Exception as exc: |
There was a problem hiding this comment.
Skip bot members in on_member_update to match bulk-sync behavior.
Bulk sync excludes bots, but this listener currently syncs them. That inconsistency can cause unnecessary writes/noise.
💡 Proposed parity fix
async def on_member_update(
self,
before: discord.Member,
after: discord.Member,
) -> None:
"""Automatically sync discord_roles when a member's roles change."""
+ if after.bot:
+ return
if before.roles == after.roles:
return🤖 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 6495 -
6516, The on_member_update listener currently processes bot accounts; update it
to early-return for bots to match bulk-sync behavior: in the on_member_update
method of the Cog, add a check (e.g., if after.bot: return) before computing
role_names and calling update_person_discord_roles so bot members are skipped
and no update_person_discord_roles(settings, str(after.id), role_names) is
attempted for bot users.
There was a problem hiding this comment.
Pull request overview
This PR improves /match-candidates by extracting only concise technical skills, adding Discord role-type classification to job requirements, and syncing Discord server roles into the candidate database to enable “skills OR role badge” matching.
Changes:
- Extend job requirement extraction to include
discord_role_typesand tighten the LLM prompt to exclude soft skills. - Update candidate search to match candidates on any required skill or any matching Discord role type, and surface
matched_discord_roles. - Add Discord role sync support (DB update helper,
/sync-discord-roles, startup bulk sync, and live sync on member role changes).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_job_match.py | Adds unit coverage for prompt changes and discord_role_types parsing/normalization/filtering. |
| tests/unit/test_candidate_search.py | Adds coverage for new early-return guard, role-only querying, matched Discord roles, and sorting. |
| packages/shared/src/five08/job_match.py | Adds Discord role constants, prompt updates, and discord_role_types extraction into JobRequirements. |
| packages/shared/src/five08/candidate_search.py | Adds role-based OR filtering, role match scoring, and matched_discord_roles on results. |
| packages/shared/src/five08/audit.py | Adds update_person_discord_roles() to update people.discord_roles by discord_user_id. |
| apps/discord_bot/src/five08/discord_bot/cogs/crm.py | Displays matched roles in /match-candidates, adds bulk and live role sync wiring + new slash command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ] | ||
|
|
||
| # Discord roles that are administrative/location/seniority rather than skills. | ||
| # These are synced but not used for skill-based role matching. |
There was a problem hiding this comment.
The comment says these roles “are synced but not used”, but the constant name and all call sites (e.g., filtering in Discord role sync) treat this set as roles to exclude from being stored/synced. Please update the comment (or rename the constant) so the behavior and documentation match.
| # These are synced but not used for skill-based role matching. | |
| # These are excluded from sync / not stored for skill-based role matching. |
| OR EXISTS ( | ||
| SELECT 1 | ||
| FROM jsonb_array_elements_text(COALESCE(p.discord_roles, '[]'::jsonb)) r | ||
| WHERE r = ANY((SELECT types FROM rtypes)) |
There was a problem hiding this comment.
The SQL WHERE clause always evaluates the JSONB role EXISTS branch, even when role_types is an empty array, which forces per-row jsonb_array_elements_text(...) work and can hurt query performance. Consider short-circuiting the role branch when no role types are provided (e.g., add an (array_length(...) IS NOT NULL AND EXISTS (...)) guard or build the WHERE clause dynamically when role_types is empty).
| OR EXISTS ( | |
| SELECT 1 | |
| FROM jsonb_array_elements_text(COALESCE(p.discord_roles, '[]'::jsonb)) r | |
| WHERE r = ANY((SELECT types FROM rtypes)) | |
| OR ( | |
| (SELECT array_length(types, 1) FROM rtypes) IS NOT NULL | |
| AND EXISTS ( | |
| SELECT 1 | |
| FROM jsonb_array_elements_text(COALESCE(p.discord_roles, '[]'::jsonb)) r | |
| WHERE r = ANY((SELECT types FROM rtypes)) | |
| ) |
| if requirements.discord_role_types: | ||
| header_parts.append( | ||
| "Role: " + ", ".join(f"`{r}`" for r in requirements.discord_role_types) | ||
| ) |
There was a problem hiding this comment.
/match-candidates now supports role-type matching, but the command still appears to hard-require requirements.required_skills (earlier in the function). That prevents role-only matches (when skills extraction returns empty but discord_role_types is populated). Update the “no skills extracted” guard to allow proceeding when either required skills or discord_role_types are present, and adjust the user-facing message accordingly.
| async def on_ready(self) -> None: | ||
| """Bulk-sync all guild member roles on startup.""" | ||
| for guild in self.bot.guilds: | ||
| try: | ||
| updated, skipped, failed = await self._bulk_sync_guild_roles(guild) | ||
| logger.info( | ||
| "Startup discord role sync: guild=%s updated=%d skipped=%d failed=%d", | ||
| guild.name, | ||
| updated, | ||
| skipped, | ||
| failed, | ||
| ) | ||
| except Exception as exc: | ||
| logger.warning( | ||
| "Startup discord role sync failed for guild %s: %s", guild.name, exc | ||
| ) | ||
|
|
There was a problem hiding this comment.
on_ready can fire multiple times (e.g., reconnects), which would rerun the full guild bulk role sync repeatedly and potentially overload the DB. Add a one-time guard (e.g., self._did_startup_role_sync) or move this to a startup hook that runs exactly once per process.
| for member in guild.members: | ||
| if member.bot: | ||
| continue | ||
| role_names = [ | ||
| r.name | ||
| for r in member.roles | ||
| if r.name not in DISCORD_ROLES_EXCLUDE_FROM_SYNC | ||
| ] | ||
| try: | ||
| did_update = await asyncio.to_thread( | ||
| update_person_discord_roles, | ||
| settings, | ||
| str(member.id), | ||
| role_names, | ||
| ) | ||
| except Exception as exc: | ||
| failed += 1 | ||
| logger.warning( | ||
| "bulk role sync: failed for user_id=%s: %s", member.id, exc | ||
| ) | ||
| continue | ||
| if did_update: | ||
| updated += 1 | ||
| else: | ||
| skipped += 1 |
There was a problem hiding this comment.
The bulk sync performs one DB update per member, awaited sequentially via asyncio.to_thread(...). On larger guilds this can take a long time and generate significant DB load. Consider batching updates (single SQL statement / executemany) or running with bounded concurrency (semaphore + gather), and/or skipping updates when the role list hasn’t changed.
| for member in guild.members: | |
| if member.bot: | |
| continue | |
| role_names = [ | |
| r.name | |
| for r in member.roles | |
| if r.name not in DISCORD_ROLES_EXCLUDE_FROM_SYNC | |
| ] | |
| try: | |
| did_update = await asyncio.to_thread( | |
| update_person_discord_roles, | |
| settings, | |
| str(member.id), | |
| role_names, | |
| ) | |
| except Exception as exc: | |
| failed += 1 | |
| logger.warning( | |
| "bulk role sync: failed for user_id=%s: %s", member.id, exc | |
| ) | |
| continue | |
| if did_update: | |
| updated += 1 | |
| else: | |
| skipped += 1 | |
| # Limit the number of concurrent DB updates to avoid overloading the DB | |
| semaphore = asyncio.Semaphore(10) | |
| async def _sync_member(member: discord.Member) -> tuple[int, int, int]: | |
| """Sync roles for a single member, returning (updated, skipped, failed).""" | |
| if member.bot: | |
| return 0, 0, 0 | |
| role_names = [ | |
| r.name | |
| for r in member.roles | |
| if r.name not in DISCORD_ROLES_EXCLUDE_FROM_SYNC | |
| ] | |
| try: | |
| async with semaphore: | |
| did_update = await asyncio.to_thread( | |
| update_person_discord_roles, | |
| settings, | |
| str(member.id), | |
| role_names, | |
| ) | |
| except Exception as exc: | |
| logger.warning( | |
| "bulk role sync: failed for user_id=%s: %s", member.id, exc | |
| ) | |
| return 0, 0, 1 | |
| if did_update: | |
| return 1, 0, 0 | |
| return 0, 1, 0 | |
| tasks = [asyncio.create_task(_sync_member(member)) for member in guild.members] | |
| results = await asyncio.gather(*tasks) | |
| for u, s, f in results: | |
| updated += u | |
| skipped += s | |
| failed += f |
| """Automatically sync discord_roles when a member's roles change.""" | ||
| if before.roles == after.roles: | ||
| return | ||
|
|
There was a problem hiding this comment.
on_member_update should skip bot accounts (similar to the bulk sync loop) to avoid unnecessary DB writes when bot roles change. Add a if after.bot: return guard before computing role_names.
| if after.bot: | |
| return |
Summary
Fixed
/match-candidatesto use looser, more effective matching by:Changes
discord_role_typesfieldmatched_discord_rolesoutputupdate_person_discord_roles()function to sync roles by discord_user_id/sync-discord-rolescommand (Steering Committee) to bulk-sync guild member roleson_member_updatelistener for automatic live role syncing/match-candidatesoutput to display matched Discord rolesTest Plan
/match-candidateson a job posting with concise skills (e.g., "effect ts", "typescript")/sync-discord-rolesto sync guild member roleson_member_update🤖 Generated with Claude Code
Summary by CodeRabbit