Skip to content

feat(gooddata-eval): register knowledge_question evaluator - #1763

Open
Tomkess wants to merge 1 commit into
masterfrom
feat/knowledge-question-evaluator
Open

feat(gooddata-eval): register knowledge_question evaluator#1763
Tomkess wants to merge 1 commit into
masterfrom
feat/knowledge-question-evaluator

Conversation

@Tomkess

@Tomkess Tomkess commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • knowledge_question fixtures were reclassified out of general_question (platform/product/policy Q&A that isn't LDM-grounded) but no evaluator was ever registered for the kind, so get_evaluator("knowledge_question") raised KeyError and every item was silently skipped — the downstream repo has kept knowledge_question: false in its test-kind config as a result.
  • Registers knowledge_question in the lazy-evaluator registry, reusing GeneralQuestionEvaluator directly rather than adding a new class: both are free-text-rubric, LLM-judged prose answers, differing only in subject matter (LDM facts vs. platform/policy facts).
  • ItemReport.test_kind is tagged from the dataset item's own test_kind field (runner.py), not from the evaluator class's test_kind attribute, so sharing one evaluator class across both kinds does not mislabel results in reports/dashboards.

Test plan

  • uv run pytest packages/gooddata-eval/tests/ — 476 passed
  • New coverage: get_evaluator("knowledge_question") resolves to GeneralQuestionEvaluator; pass/fail dispatch through the reused evaluator (test_text_evaluators.py)
  • Updated test_run_items_routes_all_supported_kinds's exact-set assertion to include knowledge_question
  • ruff check / ruff format --check clean on changed files

Summary by CodeRabbit

  • New Features

    • Added support for evaluating knowledge-question test cases using free-text, rubric-based grading.
    • Knowledge questions are now recognized as a supported evaluation type and routed through the appropriate evaluator.
  • Tests

    • Added coverage for evaluator selection and pass/fail outcomes for knowledge-question items.

knowledge_question had reclassified fixtures (commit 658b9ea6) but no
registered evaluator, so get_evaluator() raised KeyError and every item
was silently skipped -- data/test_kinds.yaml had to keep it disabled.

Reuses GeneralQuestionEvaluator directly: both are free-text-rubric,
LLM-judged prose answers, and ItemReport.test_kind is tagged from the
dataset item's own field rather than the evaluator class, so sharing
one class across both kinds doesn't mislabel results.
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8119dac6-c703-4d33-b13f-de192889b72e

📥 Commits

Reviewing files that changed from the base of the PR and between 8a7cf06 and f7d8ade.

📒 Files selected for processing (3)
  • packages/gooddata-eval/src/gooddata_eval/core/evaluators/__init__.py
  • packages/gooddata-eval/tests/test_runner.py
  • packages/gooddata-eval/tests/test_text_evaluators.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The evaluator registry adds knowledge_question as a lazy-loaded kind backed by GeneralQuestionEvaluator. Tests verify supported-kind registration, dispatch, and judge-score outcomes.

Changes

Knowledge question evaluator support

Layer / File(s) Summary
Evaluator registry routing
packages/gooddata-eval/src/gooddata_eval/core/evaluators/__init__.py
The lazy evaluator maps route knowledge_question to GeneralQuestionEvaluator.
Routing and scoring validation
packages/gooddata-eval/tests/test_runner.py, packages/gooddata-eval/tests/test_text_evaluators.py
Tests cover supported-kind registration, evaluator dispatch, and pass/fail results for knowledge-question items.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f7d8a

The change enables evaluation of knowledge questions instead of skipping them; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant get_evaluator
  participant GeneralQuestionEvaluator
  participant LLMJudge
  get_evaluator->>GeneralQuestionEvaluator: resolve knowledge_question
  GeneralQuestionEvaluator->>LLMJudge: evaluate answer
  LLMJudge-->>GeneralQuestionEvaluator: return score
Loading

Suggested reviewers: hkad98, lupko, pcerny

Poem

A rabbit found a question bright
The evaluator chose the right
A judge returned a score
Tests checked the route once more
And carrots passed with delight

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: registering the knowledge_question evaluator in gooddata-eval.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.68%. Comparing base (8a7cf06) to head (f7d8ade).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1763      +/-   ##
==========================================
+ Coverage   80.65%   80.68%   +0.02%     
==========================================
  Files         272      272              
  Lines       19369    19369              
==========================================
+ Hits        15622    15627       +5     
+ Misses       3747     3742       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant