elastic / elastic/elastic-evals-sdk-python
[kbn-evals] Code quality and dead-code cleanup
- Dominant language
- Python
- Stars
- 2
- Forks
- 0
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 18
Description
### Summary
Structural cleanups with no behaviour change.
1. URL constants and internal header builders are exported from the public API but shouldn't be. `SimpleEvaluator` and `KibanaScoresClient` are used externally but missing from `__all__`. (`__init__.py`, `api/__init__.py`)
2. Seven ES-export logging helpers in `utils/logging.py` have zero call sites in the codebase. Remove them.
3. `KibanaDatasetsClient`, `KibanaScoresClient`, and `KibanaEvaluatorsClient` each have an identical `__init__` (`kibana_url`, `api_key`, `timeout`). Extract a shared base or dataclass.
4. The connector-ID validation block is copy-pasted in `correctness/evaluator.py` and `groundedness/evaluator.py`. Extract a shared helper.
5. `runner/cli/commands/run.py` repeats env-var name strings that are already literals in `config.py`. Define them as constants in `config.py` and import them. The `_apply_overrides()` function is a slower `env.update(overrides)`. Remove it.
6. Dead `EvaluationResult.reasoning` and `.details` fields (never set or read).
7. `SimpleEvaluator` uses `@dataclass` but its `__init__` is overridden immediately, leaving an unintended `__repr__` and `__eq__`. Replace with a plain class.
8. Five always-populated fields on `EvaluationRun` are typed `Optional` for no reason. Make them required.
9. `log_evaluation_start` logs the same info that the following per-evaluator lines already cover. Remove it.
10. No tests for `runner/suites.py`, `cli/commands/run.py`, or `evaluators/criteria/*`. No `conftest.py`. `_RecordingAsyncClient` is copy-pasted into four separate test files. Move the best version to a shared fixture.
### Done when
- [ ] All items above are addressed in one PR
- [ ] No behaviour change
- [ ] Existing tests pass
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by mapping the listed files and entry points: __init__.py, api/__init__.py, utils/logging.py, correctness/evaluator.py, groundedness/evaluator.py, config.py, runner/cli/commands/run.py, and the evaluator, suite, and test modules. Inventory call sites, duplicated constructors and helpers, unused fields, and the four _RecordingAsyncClient copies before making changes. Done means all ten cleanup items are addressed in one PR, with no behavior change and existing tests passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 32/100