YAGNI cleanup and internal module renames#6
Merged
Conversation
YAGNI cleanup of the core library: - Delete unimplemented OpenAI/Anthropic/Gemini provider stubs and the ranksmith._providers re-export shim - Drop single-value config knobs: per-strategy `algorithm` literals, `pair_order_parallelism`, `score_metadata_key`, and ModelRequest.response_format/temperature (hardcoded in the Azure provider) - Keep concurrency in async strategies only: remove ThreadPoolExecutor paths from sync TourRank/AcuRank; `group_parallelism` and `batch_parallelism` now live on the async classes - Deduplicate validation helpers via strategies/_common (top_k, document length) and stop re-validating top_k in result builders - Move _benchmark/_mteb_eval/_metrics (~930 lines) to a new non-packaged benchmarks/ directory so script-support code no longer ships in the wheel; delete test-only helpers from mteb_eval - Update README/advisor skill/wiki docs and tests accordingly Verified: pytest (424 passed; 7 pre-existing lightgbm/libomp env failures), mypy clean, and live end-to-end smoke runs of all ten sync/async strategies against a local LM Studio model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rename 27 private modules across strategies/, providers/, confidence/, confidence_generation/, and confidence_training/ (e.g. strategies/_listwise.py -> strategies/listwise.py) and update every import path. Package __init__ files keep controlling the public API, so no exported name changes. Verified: pytest and mypy unchanged, plus live LM Studio smoke runs of all ten sync/async strategies producing identical rankings before and after the rename. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Six identical ensure_*_model_client guards -> one ensure_capability() - Sync/async ModelClient prompt construction shared via message builders - benchmarks: statistics.fmean, drop unused license_text parameter No behavior change: prompts, error messages, and public API identical. Verified with pytest/mypy plus an LM Studio end-to-end smoke run of all ten strategies producing identical rankings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-commit cleanup of the core library, driven by a YAGNI review. Public API behavior is unchanged except for the removal of never-implemented surface; the confidence subsystem is kept intact (only file names touched) for the upcoming CBDR work.
1. Remove speculative code and move benchmarks out of the package (
ec7fb77)OpenAIProvider,AnthropicProvider,GeminiProvider+ async forms) and theranksmith._providersre-export shim. Azure remains the only bundled provider; other vendors plug in via a customModelProvider.algorithmliterals (each accepted exactly one value; the value is still emitted in result metadata),pair_order_parallelism(async pairwise now always compares both orders concurrently),score_metadata_key(fixed to"score"), andModelRequest.response_format/temperature(hardcoded in the Azure provider).group_parallelism/batch_parallelismmoved to the async classes.top_k/document-length checks unified instrategies/common, no more triple validation per request._benchmark/_mteb_eval/_metrics(~930 lines of script-support code whose only callers are excluded from the wheel) moved to a non-packagedbenchmarks/directory; test-only helpers deleted.2. Drop leading underscores from internal module filenames (
f8b8413)Renames 27 private modules (e.g.
strategies/_listwise.py→strategies/listwise.py) across strategies/, providers/, and the three confidence packages, updating every import path. Package__init__files keep controlling the public API, and git tracks all files as renames (99–100% similarity).Net effect: −1,739 / +296 lines, wheel no longer contains stubs or benchmark modules.
Verification
pytest: 424 passed at each commit; the 7 failures are pre-existing lightgbmlibomp.dylibenvironment issues on the dev machine, unrelated to this changemypy src/: clean (45 files);ruff: clean🤖 Generated with Claude Code