fix: warm instructor mode registry to prevent thread-safety race condition by bsatapat-jpg · Pull Request #273 · lightspeed-core/lightspeed-evaluation

bsatapat-jpg · 2026-06-30T07:52:39Z

Description

Type of change

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

Assisted-by: Claude-4.6-opus-high
Generated by: Cursor

Related Tickets & Documents

Related Issue #
Closes #

Checklist before requesting a review

I have performed a self-review of my code.
PR has passed all pre-merge test jobs.
If it is a core feature, I have added thorough tests.

Testing

Please provide detailed steps to perform tests related to this code change.
How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

Bug Fixes
- Improved reliability when loading evaluation components by preloading required handlers at startup.
- Reduced the chance of intermittent failures in multi-threaded environments when components are accessed concurrently.
- Added safer handling so missing optional dependencies no longer interrupt startup.

coderabbitai · 2026-06-30T07:53:02Z

Warning

Review limit reached

@bsatapat-jpg, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b003a0fe-d72c-4f9d-a365-ee30c47b152d

📥 Commits

Reviewing files that changed from the base of the PR and between b4e954c and dbd3a28.

📒 Files selected for processing (1)

src/lightspeed_evaluation/core/metrics/ragas.py

Walkthrough

Adds a _warm_instructor_registry helper in ragas.py that eagerly iterates Instructor's mode_registry._lazy_loaders and calls get_handlers for each key at module import time, suppressing KeyError/ImportError, to prevent a thread-unsafe lazy-loading race in Instructor v1.15+.

Changes

Instructor Registry Warmup

Layer / File(s)	Summary
Import-time registry warmup `src/lightspeed_evaluation/core/metrics/ragas.py`	Defines and immediately calls `_warm_instructor_registry`, which iterates `mode_registry._lazy_loaders` and calls `mode_registry.get_handlers` for each key, suppressing `KeyError` and `ImportError` per key and swallowing top-level `ImportError` if Instructor is absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main fix: warming the Instructor mode registry to avoid a thread-safety race condition.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lightspeed_evaluation/core/metrics/ragas.py`:
- Around line 41-50: The Instructor warmup in ragas.py relies on the private
mode_registry._lazy_loaders attribute and silently swallows skipped handlers.
Update the warmup to access _lazy_loaders via getattr(mode_registry,
"_lazy_loaders", None) before iterating, and in the
mode_registry.get_handlers(...) exception path replace the silent pass with a
debug log for KeyError and ImportError so skipped warmups are visible. Use the
existing import-time warmup block around mode_registry and get_handlers to
locate the change.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d9295ffd-79ac-4511-a7a5-63350f71cc54

📥 Commits

Reviewing files that changed from the base of the PR and between ab54c98 and b4e954c.

📒 Files selected for processing (1)

src/lightspeed_evaluation/core/metrics/ragas.py

…ition

asamal4 · 2026-06-30T10:37:31Z

+logger = logging.getLogger(__name__)
+
+
+def _warm_instructor_registry() -> None:


This is another workaround - we need to spend some time to redesign this..
How do we make sure it will work always ?

The bug is in instructor>=1.15 — their ModeRegistry.get_handlers() does a non-atomic pop() + load() sequence without a lock.
Once instructlab fixes it, we can remove it out.
Wdyt?

doing a version bump here #274 - check if that resolves this issue

asamal4 · 2026-06-30T12:18:44Z

@bsatapat-jpg Can we close this PR, as per your comment the issue is fixed by the version bump in #274

bsatapat-jpg · 2026-06-30T12:52:50Z

@bsatapat-jpg Can we close this PR, as per your comment the issue is fixed by the version bump in #274

Yups it's fixed the issues. We can close it.

bsatapat-jpg changed the title ~~fix: warm instructor mode registry to prevent thread-safety race cond…~~ fix: warm instructor mode registry to prevent thread-safety race condition Jun 30, 2026

coderabbitai Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread src/lightspeed_evaluation/core/metrics/ragas.py Outdated

fix: warm instructor mode registry to prevent thread-safety race cond…

dbd3a28

…ition

bsatapat-jpg force-pushed the dev_1 branch from b4e954c to dbd3a28 Compare June 30, 2026 08:29

asamal4 reviewed Jun 30, 2026

View reviewed changes

asamal4 closed this Jun 30, 2026

bsatapat-jpg deleted the dev_1 branch July 1, 2026 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: warm instructor mode registry to prevent thread-safety race condition#273

fix: warm instructor mode registry to prevent thread-safety race condition#273
bsatapat-jpg wants to merge 1 commit into
lightspeed-core:mainfrom
bsatapat-jpg:dev_1

bsatapat-jpg commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Review limit reached

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

asamal4 Jun 30, 2026

Uh oh!

bsatapat-jpg Jun 30, 2026

Uh oh!

asamal4 Jun 30, 2026

Uh oh!

asamal4 commented Jun 30, 2026

Uh oh!

bsatapat-jpg commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		logger = logging.getLogger(__name__)


		def _warm_instructor_registry() -> None:

Uh oh!

Conversation

bsatapat-jpg commented Jun 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Tools used to create PR

Related Tickets & Documents

Checklist before requesting a review

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asamal4 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

bsatapat-jpg Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

asamal4 Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

asamal4 commented Jun 30, 2026

Uh oh!

bsatapat-jpg commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bsatapat-jpg commented Jun 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading