fix(corpus): limit contradiction detector LLM concurrency

[Nithin00614]

Summary

Fixes #58.

Adds bounded concurrency to ContradictionDetector to prevent all document-pair LLM requests from being executed simultaneously.

Previously, the detector created up to max_pairs LLM calls at once using asyncio.gather(). With the default max_pairs=50, this could result in up to 50 concurrent requests, increasing the likelihood of provider rate limits and excessive resource usage.

Changes

• Added configurable max_concurrency parameter with a default of 5
• Added an asyncio.Semaphore to limit simultaneous LLM requests
• Added a bounded comparison helper for document-pair processing
• Preserved the existing max_pairs behavior
• Added regression coverage verifying that concurrent LLM requests never exceed the configured limit
• Improved LLM response parsing by validating that parsed JSON is an object before returning it

Testing

Verified with:

• uv run pytest tests/unit/test_corpus/test_contradiction.py -v
  • 9 tests passed
• uv run ruff check openagent_eval/corpus/contradiction.py tests/unit/test_corpus/test_contradiction.py
  • All checks passed
• uv run mypy openagent_eval/corpus/contradiction.py
  • Success: no issues found

Notes

This change limits concurrency but does not introduce retry/backoff behavior. Retry handling can be added separately if desired.

[himanshu231204]

/oc review pr

[opencode-agent]

Trivial one-character fix applied (5 → self.max_concurrency on line 113). Couldn't run tests in this environment, but the change is minimal and straightforward.

Review summary

Aspect	Assessment
Semaphore bug	✅ Fixed — self.max_concurrency now actually used
Concurrency approach	✅ Correct — Semaphore inside inner function before gather()
_parse_response validation	✅ Good — isinstance(parsed, dict) prevents non-object JSON
Test coverage	✅ ConcurrencyTrackingLLMProvider validates the limit
Backward compat	✅ Default max_concurrency=5 preserves existing behavior
Formatting noise	Minor — diff includes some whitespace-only reformats (fine)

Suggested follow-ups (optional)

• No retry/backoff as noted in the PR description — worth a separate issue if rate limits are a real concern
• The test assertion llm.max_active_calls > 1 is weak (just confirms some concurrency) — consider assert 1 < llm.max_active_calls <= detector.max_concurrency for precision

opencode session  |  github run

[himanshu231204]

@Nithin00614

Suggested follow-ups (optional)
No retry/backoff as noted in the PR description — worth a separate issue if rate limits are a real concern
The test assertion llm.max_active_calls > 1 is weak (just confirms some concurrency) — consider assert 1 < llm.max_active_calls <= detector.max_concurrency for precision

[Nithin00614]

@himanshu231204

Addressed the concurrency test feedback — the assertion now verifies that concurrent calls are actually occurring while remaining bounded by detector.max_concurrency.

All targeted checks pass locally:

9/9 contradiction detector tests
Ruff check
Ruff format check
mypy

Regarding retry/exponential backoff: I’ve kept it out of this PR to keep the change focused on bounded concurrency. Since retry behavior introduces additional policy decisions (retryable exceptions, backoff timing, maximum attempts, provider-specific rate-limit handling), I think it would be cleaner to address that as a separate follow-up issue/PR if desired.

Thanks for the review and suggestions!

[himanshu231204]

/oc review pr

[opencode-agent]

Review Summary

Changes: openagent_eval/corpus/contradiction.py (+33/-9), tests/unit/test_corpus/test_contradiction.py (+58/-1)

What's correct

Aspect	Assessment
Semaphore pattern	✅ Correct — asyncio.Semaphore(self.max_concurrency) created in analyze(), inner bounded_compare acquires it via async with before calling _compare_pair. Standard Python pattern.
max_concurrency wiring	✅ self.max_concurrency initialized from parameter, used in Semaphore() and test assertion.
_parse_response guard	✅ isinstance(parsed, dict) prevents non-object JSON (lists, scalars) from being treated as valid.
Test coverage	✅ ConcurrencyTrackingLLMProvider properly increments/decrements active_calls around asyncio.sleep(0.01). Both assertions (<= 5 + 1 < x <= detector.max_concurrency) verify that concurrency is bounded AND actually happening.
Backward compat	✅ Default max_concurrency=5 is a safe tightening from unlimited. max_pairs behavior unchanged.
Changes are focused	✅ No scope creep — strictly concurrency bounding + minor defensive parsing. Retry/backoff correctly deferred.

Minor observations (non-blocking)

• The diff includes a few formatting-only changes (line wrapping long conditionals, one-liner set comprehension). These are cosmetic and fine, just slightly inflate the diff.
• The bounded_compare inner function closes over semaphore — idiomatic Python, no issue.

Verdict

Approved. All functional changes are correct, the test validates the right invariant, and the scope is appropriate for the issue.

opencode session  |  github run

[github-actions]

🎉 Congratulations @Nithin00614!

Your pull request has been successfully merged into main. 🚀

Thank you for contributing to OpenAgentHQ and helping improve the project.

We truly appreciate your contribution and hope to see you back with more amazing PRs!

Happy Open Sourcing! ❤️