diff --git a/apps/backend/src/rhesis/backend/app/routers/metric_tuning.py b/apps/backend/src/rhesis/backend/app/routers/metric_tuning.py index 25bfa08660..f5665f50ae 100644 --- a/apps/backend/src/rhesis/backend/app/routers/metric_tuning.py +++ b/apps/backend/src/rhesis/backend/app/routers/metric_tuning.py @@ -46,6 +46,7 @@ MetricTuningReviewCreate, MetricTuningRun, ) +from rhesis.backend.app.schemas.metric_tuning_metadata import MetricTuningRunSummary from rhesis.backend.app.services import metric_tuning as service from rhesis.backend.app.services.metric_tuning.invoke import MetricModelNotConfigured from rhesis.backend.app.services.metric_tuning.reviews import ( @@ -101,6 +102,25 @@ def _resolve_case_or_raise( return db_test +def _run_response( + db: Session, + metric: models.Metric, + organization_id: str, + summary: MetricTuningRunSummary, +) -> MetricTuningRun: + """The stored run summary plus the agreement, which is never stored. + + Read here on every request rather than written when a run finishes: a review + recorded between runs -- or one a run has just invalidated -- has to move the + number straight away. + """ + # Set after construction rather than passed in: the stored summary allows + # extra keys, so spreading it beside a keyword risks colliding with one. + run = MetricTuningRun(**summary.model_dump(mode="json")) + run.agreement = service.get_agreement(db, metric, organization_id) + return run + + @router.get("/{metric_id}/tuning/cases", response_model=List[MetricTuningCase]) def read_tuning_cases( metric_id: UUID, @@ -226,7 +246,7 @@ def read_tuning_run( organization_id, user_id = tenant_context metric = _resolve_metric_or_raise(db, metric_id, organization_id, user_id) summary = service.get_tuning_run(db, metric, organization_id) - return MetricTuningRun(**summary.model_dump(mode="json")) + return _run_response(db, metric, organization_id, summary) # Marked update rather than left to the POST-means-create convention: starting a @@ -278,4 +298,4 @@ def start_tuning_run( status_code=503, detail="The run could not be queued. Please try again." ) from e - return MetricTuningRun(**summary.model_dump(mode="json")) + return _run_response(db, metric, organization_id, summary) diff --git a/apps/backend/src/rhesis/backend/app/schemas/metric_tuning.py b/apps/backend/src/rhesis/backend/app/schemas/metric_tuning.py index f084806f18..68ab0ad80b 100644 --- a/apps/backend/src/rhesis/backend/app/schemas/metric_tuning.py +++ b/apps/backend/src/rhesis/backend/app/schemas/metric_tuning.py @@ -140,6 +140,34 @@ class MetricTuningCase(MetricTuningCaseBase): model_config = ConfigDict(from_attributes=True) +class TuningAgreement(BaseModel): + """How much of what the metric said the reviewer accepted. + + This is the one number an author watches while editing an evaluation prompt: + change the wording, run again, see whether it went up. + + ``ratio`` is ``None`` when nothing has been judged, never ``1.0`` -- a set + nobody has looked at has no agreement rather than a perfect one. It is + computed from the stored reviews on every read and never written down, so a + review a run has just invalidated stops counting immediately. + + ``judged`` travels with the ratio because a ratio without its denominator is + not a measurement: three out of three should not read like a solved problem. + """ + + # accepted / (accepted + rejected). None when nothing has been judged. + ratio: Optional[float] = None + # The denominator -- accepted plus rejected, and nothing else. + judged: int = 0 + accepted: int = 0 + rejected: int = 0 + # Left out of the ratio and reported beside it, never counted as accepted. + unreviewed: int = 0 + # The metric call failed. Left out too, and kept apart from the verdicts so a + # flaky provider never reads as a bad metric. + errored: int = 0 + + class MetricTuningRun(BaseModel): """The state of a metric's latest tuning run. @@ -164,6 +192,11 @@ class MetricTuningRun(BaseModel): errored_cases: int = 0 # Why the run as a whole failed. A single case failing does not fail a run. error: Optional[str] = None + # Not part of the run at all -- reviews are written between runs and change + # this without one. It travels with the run because that is the one thing the + # tab already re-reads while a run is going, and the number has to move as the + # cases land. Derived on every read; see ``TuningAgreement``. + agreement: TuningAgreement = TuningAgreement() __all__ = [ @@ -174,6 +207,7 @@ class MetricTuningRun(BaseModel): "MetricTuningReview", "MetricTuningReviewCreate", "MetricTuningRun", + "TuningAgreement", "ReviewDecision", "TuningCaseOutcome", "UnreviewedReason", diff --git a/apps/backend/src/rhesis/backend/app/services/metric_tuning/__init__.py b/apps/backend/src/rhesis/backend/app/services/metric_tuning/__init__.py index 7252d3b3cd..329acb5ade 100644 --- a/apps/backend/src/rhesis/backend/app/services/metric_tuning/__init__.py +++ b/apps/backend/src/rhesis/backend/app/services/metric_tuning/__init__.py @@ -13,6 +13,10 @@ Package façade -- callers import from here, not from the submodules. """ +from rhesis.backend.app.services.metric_tuning.agreement import ( + agreement_over, + get_agreement, +) from rhesis.backend.app.services.metric_tuning.cases import ( create_tuning_case, delete_tuning_case, @@ -62,11 +66,13 @@ "ReviewCommentRequired", "TuningRunInFlight", "accept_remaining", + "agreement_over", "case_outcome", "create_tuning_case", "delete_tuning_case", "execute_tuning_run", "fail_tuning_run", + "get_agreement", "get_or_create_tuning_test_set", "get_tuning_case", "get_tuning_run", diff --git a/apps/backend/src/rhesis/backend/app/services/metric_tuning/agreement.py b/apps/backend/src/rhesis/backend/app/services/metric_tuning/agreement.py new file mode 100644 index 0000000000..ed945407d1 --- /dev/null +++ b/apps/backend/src/rhesis/backend/app/services/metric_tuning/agreement.py @@ -0,0 +1,74 @@ +"""Agreement: the share of judged cases the reviewer accepted. + +One number for a whole tuning test set, and the only one an author watches while +editing an evaluation prompt -- change the wording, run again, see whether it +went up. + +It was originally the share of cases where the metric's verdict equalled a stored +expected verdict. There is no stored verdict any more (domain.local/adr/0005), so +it means the same thing to its reader and is computed completely differently: +accepted over accepted plus rejected, both of them a human's judgement. + +**The denominator is the whole design.** Every shortcut here inflates it. An +unreviewed case counted as accepted makes a set nobody looked at report itself +perfect; an errored case counted as rejected makes a flaky provider read as a bad +metric. Both are left out of the ratio and reported beside it instead. + +Nothing is stored. The outcomes this folds over are themselves derived from the +metric's current threshold on every read (``outcome.py``), so a review a run has +just invalidated stops counting immediately rather than at the next write. +""" + +import logging +from collections import Counter +from typing import Iterable + +from sqlalchemy.orm import Session + +from rhesis.backend.app import models +from rhesis.backend.app.crud import metric_tuning as crud_metric_tuning +from rhesis.backend.app.schemas.metric_tuning import TuningAgreement, TuningCaseOutcome +from rhesis.backend.app.schemas.metric_tuning_metadata import parse_metric_tuning_case_metadata +from rhesis.backend.app.services.metric_tuning.outcome import case_outcome +from rhesis.backend.app.services.metric_tuning.test_sets import get_tuning_test_set + +logger = logging.getLogger(__name__) + +# Places to round the ratio to. Enough for any display, and it keeps two out of +# three off the wire as 0.6667 rather than 0.6666666666666666. +RATIO_PRECISION = 4 + + +def agreement_over(outcomes: Iterable[TuningCaseOutcome]) -> TuningAgreement: + """Fold case outcomes into the ratio and the counts that qualify it.""" + counts = Counter(outcomes) + accepted = counts[TuningCaseOutcome.ACCEPTED] + rejected = counts[TuningCaseOutcome.REJECTED] + judged = accepted + rejected + + return TuningAgreement( + # Nothing judged is no agreement, not perfect agreement. + ratio=round(accepted / judged, RATIO_PRECISION) if judged else None, + judged=judged, + accepted=accepted, + rejected=rejected, + unreviewed=counts[TuningCaseOutcome.UNREVIEWED], + errored=counts[TuningCaseOutcome.ERRORED], + ) + + +def get_agreement(db: Session, metric: models.Metric, organization_id: str) -> TuningAgreement: + """The metric's agreement as its stored reviews stand right now. + + A metric with no tuning set has nothing to agree about, which is the same + all-zero, no-ratio answer as a set nobody has reviewed. + """ + test_set = get_tuning_test_set(db, metric.id, organization_id) + if not test_set: + return TuningAgreement() + + cases = crud_metric_tuning.get_tuning_cases(db, test_set.id, organization_id) + return agreement_over( + case_outcome(metric, parse_metric_tuning_case_metadata(db_test.test_metadata))[0] + for db_test in cases + ) diff --git a/apps/frontend/src/app/(protected)/metrics/[identifier]/tuning/MetricTuningTab.tsx b/apps/frontend/src/app/(protected)/metrics/[identifier]/tuning/MetricTuningTab.tsx index b1baa2bdcf..8d0ab9b375 100644 --- a/apps/frontend/src/app/(protected)/metrics/[identifier]/tuning/MetricTuningTab.tsx +++ b/apps/frontend/src/app/(protected)/metrics/[identifier]/tuning/MetricTuningTab.tsx @@ -38,6 +38,7 @@ import { ApiClientFactory } from '@/utils/api-client/client-factory'; import type { UUID } from 'crypto'; import type { ScoreType } from '@/utils/api-client/interfaces/metric'; import type { + MetricTuningAgreement, MetricTuningCase, MetricTuningCaseCreate, MetricTuningRun, @@ -58,6 +59,9 @@ const INVALIDATED_HINT = const ERRORED_HINT = 'The metric call failed for this case, so there is no verdict to judge.'; +const NO_AGREEMENT_HINT = + 'Agreement is the share of judged cases you accepted. Nothing has been judged yet, so there is no share to report — a set nobody has looked at is not a set the metric got right.'; + /** Renders long free text on one line with the full value in a tooltip. */ function TruncatedCell({ params }: { params: GridRenderCellParams }) { const value = typeof params.value === 'string' ? params.value : ''; @@ -248,6 +252,48 @@ function InvalidatedMark() { ); } +/** + * The metric's agreement, and the counts that stop it being read as more than + * it is. + * + * The denominator is the whole point. Unreviewed and errored cases are counted + * out of the ratio and reported beside it: counting either one in produces a + * plausible figure meaning something other than what its reader thinks — a set + * nobody looked at reading as perfect, or a flaky provider reading as a bad + * metric. The judged count sits next to the number for the same reason, so + * three out of three does not read like a solved problem. + */ +function AgreementSummary({ agreement }: { agreement: MetricTuningAgreement }) { + const { ratio, judged, unreviewed, errored } = agreement; + const total = judged + unreviewed + errored; + + const qualifiers = [ + judged > 0 + ? `over ${judged} of ${total} ${total === 1 ? 'case' : 'cases'} judged` + : 'nothing judged yet', + unreviewed > 0 ? `${unreviewed} unreviewed` : null, + errored > 0 ? `${errored} the metric could not be reached on` : null, + ].filter(Boolean); + + return ( + + + Agreement + + + {ratio === null ? '—' : `${Math.round(ratio * 100)}%`} + + + {qualifiers.join(' · ')} + + + ); +} + /** One line about the latest run, or nothing when there has not been one. */ function RunSummary({ run }: { run: MetricTuningRun | null }) { if (!run || run.status === 'never_run') return null; @@ -443,6 +489,18 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { [metricId, notifications] ); + // Agreement is derived from the reviews, so judging a case moves it without a + // run. Re-read rather than recomputed here: one fold, on the server, is what + // stops the number and the rows it is folded from ever disagreeing. + const refreshRun = useCallback(async () => { + try { + const client = new ApiClientFactory().getMetricTuningClient(); + setRun(await client.getTuningRun(metricId)); + } catch { + // The line keeps its last value; the next review or poll corrects it. + } + }, [metricId]); + /** Swaps in the case the review endpoint returns, leaving the rest alone. */ const replaceCase = useCallback((updated: MetricTuningCase) => { setCases(prev => @@ -459,6 +517,7 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { decision: 'accepted', }) ); + await refreshRun(); } catch (error) { notifications.show( error instanceof Error @@ -468,7 +527,7 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { ); } }, - [metricId, notifications, replaceCase] + [metricId, notifications, refreshRun, replaceCase] ); // Throws on failure so the dialog keeps the comment on screen — a rejection @@ -483,8 +542,9 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { comment, }) ); + await refreshRun(); }, - [metricId, rejecting, replaceCase] + [metricId, refreshRun, rejecting, replaceCase] ); const handleAcceptRest = useCallback(async () => { @@ -492,6 +552,7 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { try { const client = new ApiClientFactory().getMetricTuningClient(); setCases(await client.acceptRemainingTuningCases(metricId)); + await refreshRun(); notifications.show('Accepted every case left unreviewed', { severity: 'success', autoHideDuration: 4000, @@ -506,7 +567,7 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { } finally { setAcceptingRest(false); } - }, [metricId, notifications]); + }, [metricId, notifications, refreshRun]); const openAdd = useCallback(() => { setEditing(null); @@ -707,6 +768,13 @@ export default function MetricTuningTab({ metricId }: MetricTuningTabProps) { /> ) : ( <> + {/* Nothing while a run is going: until the worker has cleared the + last run's results, the number is the previous run's, and one + sitting above a progress line reads as this run's. The progress + line says what is happening instead. */} + {hasResults && run && !isRunning && ( + + )} +): MetricTuningAgreement => ({ ...NO_AGREEMENT, ...fields }); + const NEVER_RUN: MetricTuningRun = { status: 'never_run', started_at: null, @@ -86,6 +101,7 @@ const NEVER_RUN: MetricTuningRun = { completed_cases: 0, errored_cases: 0, error: null, + agreement: NO_AGREEMENT, }; describe('MetricTuningTab', () => { @@ -238,6 +254,7 @@ describe('MetricTuningTab — runs', () => { completed_cases: 1, errored_cases: 0, error: null, + agreement: NO_AGREEMENT, }; const COMPLETED: MetricTuningRun = { @@ -248,6 +265,7 @@ describe('MetricTuningTab — runs', () => { completed_cases: 1, errored_cases: 0, error: null, + agreement: NO_AGREEMENT, }; const SCORED_CASE: MetricTuningCase = { @@ -710,3 +728,127 @@ describe('MetricTuningTab — reviewing', () => { ).toBeDisabled(); }); }); + +describe('MetricTuningTab — agreement', () => { + /** A finished run whose agreement is whatever the test needs it to be. */ + const runWith = ( + fields: Partial + ): MetricTuningRun => ({ + ...NEVER_RUN, + status: 'completed', + started_at: '2026-08-13T10:00:00Z', + completed_at: '2026-08-13T10:01:00Z', + total_cases: 3, + completed_cases: 3, + agreement: agreement(fields), + }); + + beforeEach(() => { + jest.clearAllMocks(); + mockGetMetric.mockResolvedValue(BINARY_METRIC); + mockGetTuningCases.mockResolvedValue([JUDGEABLE_CASE]); + mockGetTuningRun.mockResolvedValue(NEVER_RUN); + }); + + it('shows the number and the count it was computed over', async () => { + mockGetTuningRun.mockResolvedValue( + runWith({ ratio: 0.6667, judged: 3, accepted: 2, rejected: 1 }) + ); + + render(); + + expect(await screen.findByText('67%')).toBeInTheDocument(); + // The denominator travels with it: three out of three must not read like a + // solved problem. + expect(screen.getByText(/over 3 of 3 cases judged/i)).toBeInTheDocument(); + }); + + it('reports no agreement rather than full agreement when nothing is judged', async () => { + mockGetTuningRun.mockResolvedValue(runWith({ unreviewed: 3 })); + + render(); + + expect(await screen.findByText(/nothing judged yet/i)).toBeInTheDocument(); + expect(screen.queryByText('100%')).not.toBeInTheDocument(); + expect( + screen.getByTitle(/nothing has been judged yet/i) + ).toBeInTheDocument(); + }); + + it('counts unreviewed cases out of the ratio and reports them beside it', async () => { + mockGetTuningRun.mockResolvedValue( + runWith({ ratio: 1, judged: 1, accepted: 1, unreviewed: 2 }) + ); + + render(); + + expect(await screen.findByText('100%')).toBeInTheDocument(); + expect(screen.getByText(/over 1 of 3 cases judged/i)).toBeInTheDocument(); + expect(screen.getByText(/2 unreviewed/i)).toBeInTheDocument(); + }); + + it('reports errored cases apart, so a flaky provider is visibly one', async () => { + mockGetTuningRun.mockResolvedValue( + runWith({ ratio: 1, judged: 1, accepted: 1, errored: 2 }) + ); + + render(); + + expect(await screen.findByText('100%')).toBeInTheDocument(); + expect( + screen.getByText(/2 the metric could not be reached on/i) + ).toBeInTheDocument(); + }); + + it('says nothing about agreement before the metric has been run', async () => { + mockGetTuningCases.mockResolvedValue([CASE]); + + render(); + + await screen.findByText('How are you?'); + expect(screen.queryByText('Agreement')).not.toBeInTheDocument(); + }); + + it('re-reads the agreement after a review, since judging a case moves it', async () => { + mockGetTuningRun + .mockResolvedValueOnce(runWith({ unreviewed: 1 })) + .mockResolvedValue(runWith({ ratio: 1, judged: 1, accepted: 1 })); + mockReviewTuningCase.mockResolvedValue({ + ...JUDGEABLE_CASE, + outcome: 'accepted', + unreviewed_reason: null, + review: { + decision: 'accepted', + comment: null, + verdict: 'pass', + reviewed_at: '2026-08-14T09:00:00Z', + }, + }); + + render(); + await screen.findByText(/nothing judged yet/i); + + fireEvent.click( + screen.getByRole('button', { name: /accept this verdict/i }) + ); + + expect(await screen.findByText('100%')).toBeInTheDocument(); + }); + + it('shows no number while a run is going, since it is about to change', async () => { + // Until the worker clears the last run's results this is the *previous* + // run's number, and one sitting above a progress line reads as this run's. + mockGetTuningRun.mockResolvedValue({ + ...runWith({ ratio: 1, judged: 3, accepted: 3 }), + status: 'running', + completed_at: null, + completed_cases: 1, + }); + + render(); + + expect(await screen.findByText(/1 done/i)).toBeInTheDocument(); + expect(screen.queryByText('Agreement')).not.toBeInTheDocument(); + expect(screen.queryByText('100%')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/frontend/src/utils/api-client/interfaces/metric-tuning.ts b/apps/frontend/src/utils/api-client/interfaces/metric-tuning.ts index f7bd07229e..a78a8ad8f5 100644 --- a/apps/frontend/src/utils/api-client/interfaces/metric-tuning.ts +++ b/apps/frontend/src/utils/api-client/interfaces/metric-tuning.ts @@ -66,6 +66,27 @@ export interface MetricTuningCase { */ export type TuningRunStatus = 'never_run' | 'running' | 'completed' | 'failed'; +/** + * How much of what the metric said the reviewer accepted — the one number to + * watch while editing an evaluation prompt. + * + * `ratio` is null when nothing has been judged, never 1 — a set nobody has + * looked at has no agreement rather than a perfect one. Unreviewed and errored + * cases are counted out of it and reported beside it instead. + */ +export interface MetricTuningAgreement { + /** accepted / (accepted + rejected), or null when nothing has been judged. */ + ratio: number | null; + /** The denominator. Never show the ratio without it. */ + judged: number; + accepted: number; + rejected: number; + /** Left out of the ratio, never counted as accepted. */ + unreviewed: number; + /** The metric call failed — left out too, and reported apart. */ + errored: number; +} + /** * A metric's latest tuning run. Only the latest is kept — a new run overwrites * the previous one. @@ -81,6 +102,11 @@ export interface MetricTuningRun { errored_cases: number; /** Why the run as a whole failed. One case failing does not fail a run. */ error: string | null; + /** + * Recomputed from the stored reviews on every read, so a review recorded + * between runs moves it without a run. + */ + agreement: MetricTuningAgreement; } export interface MetricTuningCaseCreate { diff --git a/tests/backend/routes/test_metric_tuning_runs.py b/tests/backend/routes/test_metric_tuning_runs.py index b2937dadbf..77be627cf0 100644 --- a/tests/backend/routes/test_metric_tuning_runs.py +++ b/tests/backend/routes/test_metric_tuning_runs.py @@ -4,7 +4,9 @@ - POST /metrics/{metric_id}/tuning/run - GET /metrics/{metric_id}/tuning/run -plus what a run leaves behind on the case list. +plus what a run leaves behind on the case list, and the agreement the run +endpoint reports over it -- which is derived from the stored reviews on every +read rather than written down anywhere. The metric invocation is stubbed throughout, which is what makes these deterministic and free of LLM calls. Celery is stubbed too: this codebase tests @@ -1008,6 +1010,196 @@ def test_changing_the_score_type_invalidates_every_review( ] +@pytest.mark.integration +@pytest.mark.routes +class TestAgreement: + """The one number for the whole set, read off GET /tuning/run. + + Agreement is the share of judged cases the reviewer accepted. Nothing is + compared for equality — there is no stored expected verdict to compare + against (ADR-0005) — so every one of these goes through a real review. + + The denominator is what these are actually about. Counting unreviewed cases + as accepted, or errored ones as rejected, both produce a plausible figure + that means something other than what its reader thinks. + """ + + def _agreement(self, client: TestClient, metric_id) -> dict: + response = client.get(f"/metrics/{metric_id}/tuning/run") + assert response.status_code == status.HTTP_200_OK, response.text + return response.json()["agreement"] + + def test_agreement_is_accepted_over_accepted_plus_rejected( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + tuning_metric: models.Metric, + ): + kept = _create_case(authenticated_client, tuning_metric.id) + wrong = _create_case(authenticated_client, tuning_metric.id, input="The second one") + _run( + test_db, + tuning_metric, + test_org_id, + by_input={ + CASE_INPUT: {"score": 1.0, "reason": "fine"}, + "The second one": {"score": 0.0, "reason": "toxic"}, + }, + ) + _review(authenticated_client, tuning_metric.id, kept["id"], "accepted") + _review(authenticated_client, tuning_metric.id, wrong["id"], "rejected", REVIEW_COMMENT) + + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] == 0.5 + assert agreement["judged"] == 2 + assert agreement["accepted"] == 1 + assert agreement["rejected"] == 1 + + def test_the_judged_count_travels_with_the_ratio( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + tuning_metric: models.Metric, + ): + """Three out of three must not read like a solved problem.""" + for index in range(3): + _create_case(authenticated_client, tuning_metric.id, input=f"Case {index}") + _run( + test_db, + tuning_metric, + test_org_id, + *[{"score": 1.0, "reason": "fine"}] * 3, + ) + authenticated_client.post(f"/metrics/{tuning_metric.id}/tuning/reviews/accept-rest") + + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] == 1.0 + assert agreement["judged"] == 3 + + def test_a_set_nobody_has_reviewed_has_no_agreement_rather_than_full_agreement( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + tuning_metric: models.Metric, + ): + _create_case(authenticated_client, tuning_metric.id) + _run(test_db, tuning_metric, test_org_id, {"score": 1.0, "reason": "fine"}) + + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] is None + assert agreement["judged"] == 0 + assert agreement["unreviewed"] == 1 + + def test_unreviewed_cases_are_left_out_of_the_ratio_and_reported_beside_it( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + tuning_metric: models.Metric, + ): + """Counting them in is what makes a set nobody looked at report itself perfect.""" + judged = _create_case(authenticated_client, tuning_metric.id) + _create_case(authenticated_client, tuning_metric.id, input="The second one") + _create_case(authenticated_client, tuning_metric.id, input="The third one") + _run( + test_db, + tuning_metric, + test_org_id, + *[{"score": 0.0, "reason": "toxic"}] * 3, + ) + _review(authenticated_client, tuning_metric.id, judged["id"], "rejected", REVIEW_COMMENT) + + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] == 0.0 + assert agreement["judged"] == 1 + assert agreement["unreviewed"] == 2 + assert agreement["accepted"] == 0 + + def test_errored_cases_are_reported_apart_rather_than_read_as_disagreement( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + tuning_metric: models.Metric, + ): + """A flaky provider must never drag the number down.""" + reached = _create_case(authenticated_client, tuning_metric.id) + _create_case(authenticated_client, tuning_metric.id, input="The second one") + _run( + test_db, + tuning_metric, + test_org_id, + by_input={ + CASE_INPUT: {"score": 1.0, "reason": "fine"}, + "The second one": RuntimeError("provider unreachable"), + }, + ) + _review(authenticated_client, tuning_metric.id, reached["id"], "accepted") + + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] == 1.0 + assert agreement["judged"] == 1 + assert agreement["errored"] == 1 + assert agreement["rejected"] == 0 + assert agreement["unreviewed"] == 0 + + def test_a_metric_that_has_never_been_run_has_no_agreement( + self, authenticated_client: TestClient, tuning_metric: models.Metric + ): + """One shape either way, so the tab has nothing to branch on.""" + agreement = self._agreement(authenticated_client, tuning_metric.id) + + assert agreement["ratio"] is None + assert agreement["judged"] == 0 + + def test_a_review_a_re_run_invalidated_stops_counting_at_once( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + numeric_metric: models.Metric, + ): + """Nothing is cached, so the drop shows on the next read rather than the + next write. A held-over number is the shortcut this rules out.""" + case = _create_case(authenticated_client, numeric_metric.id) + _run(test_db, numeric_metric, test_org_id, {"score": 0.79, "reason": "mostly relevant"}) + _review(authenticated_client, numeric_metric.id, case["id"], "accepted") + assert self._agreement(authenticated_client, numeric_metric.id)["ratio"] == 1.0 + + _run(test_db, numeric_metric, test_org_id, {"score": 0.2, "reason": "off topic now"}) + + agreement = self._agreement(authenticated_client, numeric_metric.id) + assert agreement["ratio"] is None + assert agreement["judged"] == 0 + assert agreement["unreviewed"] == 1 + + def test_a_review_that_survived_a_re_run_keeps_counting( + self, + authenticated_client: TestClient, + test_db: Session, + test_org_id, + numeric_metric: models.Metric, + ): + """The other half of the same rule: ordinary drift is not a change.""" + case = _create_case(authenticated_client, numeric_metric.id) + _run(test_db, numeric_metric, test_org_id, {"score": 0.79, "reason": "mostly relevant"}) + _review(authenticated_client, numeric_metric.id, case["id"], "accepted") + + _run(test_db, numeric_metric, test_org_id, {"score": 0.81, "reason": "still relevant"}) + + agreement = self._agreement(authenticated_client, numeric_metric.id) + assert agreement["ratio"] == 1.0 + assert agreement["judged"] == 1 + + @pytest.mark.integration @pytest.mark.routes class TestARunThatNeverFinishes: diff --git a/tests/backend/services/metric_tuning/test_agreement.py b/tests/backend/services/metric_tuning/test_agreement.py new file mode 100644 index 0000000000..392e614fd8 --- /dev/null +++ b/tests/backend/services/metric_tuning/test_agreement.py @@ -0,0 +1,93 @@ +"""Unit tests for the agreement fold — accepted over accepted plus rejected. + +The denominator is the whole design, so these drive it directly rather than +through a run: every tempting shortcut inflates it, and each one is a case here. +A pure function over outcomes, so no database and no metric. + +Run with: python -m pytest tests/backend/services/metric_tuning/test_agreement.py -v +""" + +import pytest + +from rhesis.backend.app.schemas.metric_tuning import TuningCaseOutcome +from rhesis.backend.app.services.metric_tuning.agreement import agreement_over + +ACCEPTED = TuningCaseOutcome.ACCEPTED +REJECTED = TuningCaseOutcome.REJECTED +ERRORED = TuningCaseOutcome.ERRORED +UNREVIEWED = TuningCaseOutcome.UNREVIEWED + + +@pytest.mark.unit +class TestTheRatio: + def test_agreement_is_accepted_over_accepted_plus_rejected(self): + agreement = agreement_over([ACCEPTED, ACCEPTED, ACCEPTED, REJECTED]) + + assert agreement.ratio == 0.75 + assert agreement.judged == 4 + + def test_everything_accepted_is_full_agreement(self): + assert agreement_over([ACCEPTED, ACCEPTED]).ratio == 1.0 + + def test_everything_rejected_is_none_of_it(self): + assert agreement_over([REJECTED, REJECTED]).ratio == 0.0 + + def test_a_repeating_ratio_is_rounded_rather_than_sent_raw(self): + """0.6667 reads as a share. 0.6666666666666666 reads as a bug.""" + assert agreement_over([ACCEPTED, ACCEPTED, REJECTED]).ratio == 0.6667 + + +@pytest.mark.unit +class TestWhatIsLeftOutOfTheDenominator: + def test_an_unreviewed_case_is_not_counted_as_accepted(self): + """The shortcut that makes a set nobody looked at report itself perfect.""" + agreement = agreement_over([ACCEPTED, UNREVIEWED, UNREVIEWED]) + + assert agreement.ratio == 1.0 + assert agreement.judged == 1 + assert agreement.unreviewed == 2 + + def test_an_errored_case_is_not_counted_as_rejected(self): + """The shortcut that makes a flaky provider read as a bad metric.""" + agreement = agreement_over([ACCEPTED, ERRORED, ERRORED]) + + assert agreement.ratio == 1.0 + assert agreement.judged == 1 + assert agreement.errored == 2 + + def test_nothing_judged_has_no_agreement_rather_than_full_agreement(self): + agreement = agreement_over([UNREVIEWED, UNREVIEWED, ERRORED]) + + assert agreement.ratio is None + assert agreement.judged == 0 + + def test_no_cases_at_all_has_no_agreement(self): + agreement = agreement_over([]) + + assert agreement.ratio is None + assert agreement.judged == 0 + assert agreement.unreviewed == 0 + assert agreement.errored == 0 + + +@pytest.mark.unit +class TestTheCountsBesideIt: + def test_every_case_lands_in_exactly_one_count(self): + """The four never collapse into fewer, and none of them double-count.""" + outcomes = [ACCEPTED, ACCEPTED, REJECTED, UNREVIEWED, ERRORED] + + agreement = agreement_over(outcomes) + + assert agreement.accepted == 2 + assert agreement.rejected == 1 + assert agreement.unreviewed == 1 + assert agreement.errored == 1 + assert ( + agreement.accepted + agreement.rejected + agreement.unreviewed + agreement.errored + == len(outcomes) + ) + + def test_judged_is_the_denominator_and_nothing_else(self): + agreement = agreement_over([ACCEPTED, REJECTED, UNREVIEWED, ERRORED]) + + assert agreement.judged == 2