6krrt/tests/test_feedback.py

"""Tests for feedback.py — folding observed outcomes back into proficiency.

Structural and local_llm verdicts no longer feed proficiency (they bias
outcome_score downward by checker frequency). Client outcomes (kind='client_outcome')
are the only two-way evidence: 'succeeded' as positive, 'failed' as negative,
both accumulating through add_outcome.
"""

import sqlite3
import sys
from pathlib import Path
from unittest.mock import patch

import pytest

import feedback
from config import load_config
from feedback import (
    FAILURE_VERDICTS,
    apply_failures,
    coverage,
    summarize,
    unapplied_failures,
)
import proficiency_store

ROOT = Path(__file__).resolve().parent.parent
SCHEMA_SQL = (ROOT / "config" / "schema.sql").read_text()
CFG = load_config(ROOT / "config" / "config.yaml")


@pytest.fixture
def db(tmp_path):
    conn = sqlite3.connect(tmp_path / "t.db")
    conn.row_factory = sqlite3.Row
    conn.executescript(SCHEMA_SQL)
    conn.execute(
        """
        INSERT INTO models (model_id, provider, base_model_id, availability, last_updated)
        VALUES ('m', 'nw', 'm', 'active', '2026-08-17T00:00:00+00:00')
        """
    )
    conn.commit()
    yield conn
    conn.close()


def _verify(conn, verdict, category="coding_general", kind="structural"):
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', ?, ?, ?, '2026-08-17T00:00:00+00:00')
        """,
        (category, kind, verdict),
    )
    conn.commit()


def _prof(conn):
    return conn.execute(
        "SELECT outcome_score s, outcome_samples n FROM proficiency WHERE model_id='m'"
    ).fetchone()


# --- only failures are folded in ------------------------------------------

def test_passes_are_not_recorded_as_samples(db):
    # A structural 'ok' means "it parsed", not "it was correct". Recording it
    # as a 1.0 would inflate every score toward the ceiling.
    for _ in range(5):
        _verify(db, "ok")
    assert unapplied_failures(db) == []


def test_unverifiable_is_not_evidence(db):
    # The checker had nothing to say; that says nothing about the model
    for _ in range(5):
        _verify(db, "unverifiable")
    assert unapplied_failures(db) == []


@pytest.mark.parametrize("verdict", FAILURE_VERDICTS)
def test_failures_are_collected(db, verdict):
    _outcome(db, verdict)
    assert len(unapplied_failures(db)) == 1


def test_a_failure_drags_the_score_down_proportionally(db):
    from proficiency_store import add_outcome

    # Given: model has 3 success samples at 1.0
    add_outcome(db, CFG, "m", "nw", "coding_general", [1.0] * 3)
    _outcome(db, "failed")

    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    row = _prof(db)
    # One 0.0 folded into the running mean: (1.0*3 + 0.0)/4
    assert row["n"] == 4
    assert row["s"] == pytest.approx(0.75)


def test_a_model_that_never_fails_keeps_its_score(db):
    from proficiency_store import add_outcome

    add_outcome(db, CFG, "m", "nw", "coding_general", [1.0] * 3)
    for _ in range(20):
        _verify(db, "ok")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    row = _prof(db)
    assert (row["n"], row["s"]) == (3, pytest.approx(1.0))


# --- idempotence ----------------------------------------------------------

def test_a_failure_is_applied_only_once(db):
    _outcome(db, "failed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    first_n = _prof(db)["n"]
    # Re-running must not penalize the model again for the same bad response
    assert unapplied_failures(db) == []
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    assert _prof(db)["n"] == first_n


def test_dry_run_changes_nothing(db):
    _outcome(db, "failed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=True)
    assert _prof(db) is None
    assert len(unapplied_failures(db)) == 1


# --- grouping -------------------------------------------------------------

def test_failures_group_by_model_and_category(db):
    _outcome(db, "failed", category="coding_general")
    _outcome(db, "failed", category="coding_general")
    _outcome(db, "failed", category="debugging")
    grouped = summarize(unapplied_failures(db))
    assert {k[2]: len(v) for k, v in grouped.items()} == {
        "coding_general": 2,
        "debugging": 1,
    }


def test_failures_without_a_category_are_skipped(db):
    # Nothing to attribute them to — proficiency is per-category
    db.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', NULL, 'structural', 'failed', '2026-08-17T00:00:00+00:00')
        """
    )
    db.commit()
    assert unapplied_failures(db) == []


def test_both_check_kinds_count(db):
    _outcome(db, "failed", kind="structural")
    _outcome(db, "failed", kind="local_llm")
    assert len(unapplied_failures(db)) == 2


# --- attribution: not every failure is the model's fault -------------------

def _verify_capped(conn, verdict="failed", category="coding_general"):
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict,
                                   observed_at, model_attributable)
        VALUES ('m', 'nw', ?, 'structural', ?, '2026-08-17T00:00:00+00:00', 0)
        """,
        (category, verdict),
    )
    conn.commit()


def test_client_capped_failure_is_not_the_models_fault(db):
    # Found by forcing it: a request with max_tokens=40 fails, and without
    # this any agent using a tight cap would systematically drag down whatever
    # model it routed to.
    _verify_capped(db)
    assert unapplied_failures(db) == []


def test_capped_failures_are_still_recorded_for_visibility(db):
    # The response really was unusable — it just says nothing about the model
    _verify_capped(db)
    n = db.execute("SELECT COUNT(*) c FROM verifications WHERE verdict='failed'").fetchone()["c"]
    assert n == 1


def test_attributable_and_capped_failures_are_separated(db):
    _outcome(db, "failed")      # model's fault
    _verify_capped(db)          # client's cap
    rows = unapplied_failures(db)
    assert len(rows) == 1
    assert rows[0]["verdict"] == "failed"


# --- client outcomes: the only two-way evidence ----------------------------

def _outcome(conn, verdict, category="coding_general", kind="client_outcome"):
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', ?, ?, ?, '2026-08-17T00:00:00+00:00')
        """,
        (category, kind, verdict),
    )
    conn.commit()


def test_a_client_reported_failure_counts_against(db):
    from proficiency_store import add_outcome

    add_outcome(db, CFG, "m", "nw", "coding_general", [1.0] * 3)
    _outcome(db, "failed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    assert _prof(db)["s"] == pytest.approx(0.75)


def test_a_structural_pass_contributes_nothing(db):
    # Structural ok only means code parsed — weak evidence against recording.
    # outcome_score should not budge when only structural passes exist.
    for _ in range(10):
        _verify(db, "ok")
    assert unapplied_failures(db) == []


def test_successes_and_failures_mix_into_one_rate(db):
    _outcome(db, "succeeded")
    _outcome(db, "succeeded")
    _outcome(db, "succeeded")
    _outcome(db, "failed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    row = _prof(db)
    assert (row["n"], row["s"]) == (4, pytest.approx(0.75))


# --- structural/local_llm verdicts do NOT move proficiency ------------------

@pytest.mark.parametrize("verdict", ("truncated", "malformed"))
def test_structural_verdicts_not_collected(db, verdict):
    # Structural check verdicts are diagnostics only — "it was truncated" or
    # "it was malformed" cannot be attributed to model quality without the
    # client confirming the job failed.
    _verify(db, verdict)
    assert unapplied_failures(db) == []


@pytest.mark.parametrize("verdict", ("truncated", "malformed"))
def test_local_llm_verdicts_not_collected(db, verdict):
    # local_llm checks use the same diagnostic-verdicts; they should not move
    # outcome_score even though coverage() still reports them.
    _verify(db, verdict, kind="local_llm")
    assert unapplied_failures(db) == []


def test_structural_verdict_does_not_budge_score(db):
    # Given: model with outcome_score from client outcomes
    from proficiency_store import add_outcome

    add_outcome(db, CFG, "m", "nw", "coding_general", [1.0] * 3)
    # structural checks produce "truncated"/"malformed" — these should NOT be
    # folded into proficiency at all.
    for _ in range(10):
        _verify(db, "malformed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    row = _prof(db)
    assert row["n"] == 3
    assert row["s"] == pytest.approx(1.0)


# --- client outcomes route to add_outcome (not add_self_eval) ---------------

@pytest.mark.parametrize("verdict", ("succeeded", "failed"))
def test_client_outcomes_route_to_add_outcome(db, verdict):
    """apply_failures must call add_outcome for client outcome rows."""
    with patch("feedback.add_outcome") as mock_add_outcome:
        _outcome(db, verdict)
        apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
        mock_add_outcome.assert_called_once()
        call_args = mock_add_outcome.call_args
        assert call_args[0][0] is db          # conn
        assert call_args[0][1] is CFG         # cfg
        assert call_args[0][2:5] == ("m", "nw", "coding_general")  # model, provider, cat
        expected_score = 1.0 if verdict == "succeeded" else 0.0
        assert call_args[0][5] == [expected_score]  # scores


def test_client_outcome_applied_at_idempotent(db):
    """Running feedback.py twice applies the outcome only once."""
    _outcome(db, "failed")
    assert len(unapplied_failures(db)) == 1
    row_before = _prof(db)
    assert row_before is None

    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    row_after = _prof(db)
    assert row_after["n"] == 1
    assert row_after["s"] == pytest.approx(0.0)

    # Second run: applied_at is set, unapplied_failures returns [].
    assert unapplied_failures(db) == []
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=False)
    assert _prof(db)["n"] == row_after["n"]


# --- --dry-run applies nothing ---------------------------------------------

def test_dry_run_does_not_call_add_outcome(db):
    """A dry-run invocation must not call add_outcome at all."""
    with patch("feedback.add_outcome") as mock_add_outcome:
        _outcome(db, "failed")
        apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=True)
        mock_add_outcome.assert_not_called()


def test_dry_run_preserves_verifications_unapplied(db):
    """After dry_run, the verifications row still has applied_at IS NULL."""
    _outcome(db, "failed")
    apply_failures(db, CFG, summarize(unapplied_failures(db)), dry_run=True)
    # The prof table was never touched and the verifications row is still
    # flagged as unapplied.
    assert _prof(db) is None
    has_unapplied = db.execute(
        "SELECT COUNT(*) n FROM verifications WHERE verdict='failed' AND applied_at IS NULL"
    ).fetchone()["n"]
    assert has_unapplied == 1


# --- coverage() reports all verdicts including diagnostics ------------------

def test_coverage_reports_all_verdicts(db, capfd):
    """coverage() must still show structural/local_llm so diagnostics are visible."""
    _verify(db, "ok")
    _verify(db, "malformed")
    _verify(db, "truncated", kind="local_llm")
    _outcome(db, "succeeded")
    _outcome(db, "failed")
    coverage(db)
    captured = capfd.readouterr()
    assert "ok" in captured.out
    assert "malformed" in captured.out
    assert "truncated" in captured.out
    assert "succeeded" in captured.out
    assert "failed" in captured.out
    assert "unverifiable" not in captured.out


# --- feedback.py CLI contract (used by /admin/api/apply-feedback) ------------


def _make_main_db(tmp_path):
    """Build a DB suitable for exercising ``feedback.main()`` offline."""
    path = tmp_path / "feedback.db"
    conn = sqlite3.connect(str(path))
    conn.row_factory = sqlite3.Row
    conn.executescript(SCHEMA_SQL)
    conn.execute(
        """
        INSERT INTO models (model_id, provider, base_model_id, availability, last_updated)
        VALUES ('m', 'nw', 'm', 'active', '2026-08-17T00:00:00+00:00')
        """
    )
    conn.commit()
    conn.close()
    return path


def test_main_contract_dry_run_does_not_apply(monkeypatch, tmp_path, capsys):
    """--dry-run prints the summary and leaves verifications unapplied."""
    db_path = _make_main_db(tmp_path)
    cfg = load_config(str(ROOT / "config" / "config.yaml"))
    monkeypatch.setattr(cfg.database, "path", str(db_path))
    monkeypatch.setattr(feedback, "load_config", lambda _path: cfg)

    # Seed one unapplied client failure.
    conn = sqlite3.connect(str(db_path))
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', 'coding_general', 'client_outcome', 'failed', '2026-08-17T00:00:00+00:00')
        """
    )
    conn.commit()
    conn.close()

    with patch(
        "proficiency_store.add_outcome",
        wraps=proficiency_store.add_outcome,
    ) as mock_add_outcome:
        monkeypatch.setattr(sys, "argv", ["feedback", "--dry-run"])
        assert feedback.main() == 0
        # The dry-run path exercises add_outcome through fold_onto on an
        # in-memory copy — the mock confirms the real path is hit.
        mock_add_outcome.assert_called()

    # Verify the on-disk database was not modified.
    conn = sqlite3.connect(str(db_path))
    conn.row_factory = sqlite3.Row
    row = conn.execute(
        "SELECT applied_at FROM verifications WHERE verdict = 'failed'"
    ).fetchone()
    assert row is not None
    assert row["applied_at"] is None
    conn.close()

    captured = capsys.readouterr().out
    assert "coding_general" in captured
    assert "would apply" in captured


def test_main_contract_applies_client_outcomes_via_add_outcome(monkeypatch, tmp_path, capsys):
    """The CLI route applies client outcomes through add_outcome."""
    db_path = _make_main_db(tmp_path)
    cfg = load_config(str(ROOT / "config" / "config.yaml"))
    monkeypatch.setattr(cfg.database, "path", str(db_path))
    monkeypatch.setattr(feedback, "load_config", lambda _path: cfg)

    # Seed one unapplied client success.
    conn = sqlite3.connect(str(db_path))
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', 'coding_general', 'client_outcome', 'succeeded', '2026-08-17T00:00:00+00:00')
        """
    )
    conn.commit()
    conn.close()

    monkeypatch.setattr(sys, "argv", ["feedback"])
    assert feedback.main() == 0

    captured = capsys.readouterr().out
    assert "applying" in captured
    assert "applied" in captured.split("applying")[-1]

    conn = sqlite3.connect(str(db_path))
    conn.row_factory = sqlite3.Row
    row = conn.execute(
        "SELECT outcome_score s, outcome_samples n FROM proficiency WHERE model_id='m'"
    ).fetchone()
    conn.close()
    assert row is not None
    assert row["n"] == 1
    assert row["s"] == pytest.approx(1.0)


def test_main_contract_idempotent(monkeypatch, tmp_path):
    """A second non-dry run sees no unapplied signals and is a no-op."""
    db_path = _make_main_db(tmp_path)
    cfg = load_config(str(ROOT / "config" / "config.yaml"))
    monkeypatch.setattr(cfg.database, "path", str(db_path))
    monkeypatch.setattr(feedback, "load_config", lambda _path: cfg)

    conn = sqlite3.connect(str(db_path))
    conn.execute(
        """
        INSERT INTO verifications (model_id, provider, task_category, kind, verdict, observed_at)
        VALUES ('m', 'nw', 'coding_general', 'client_outcome', 'failed', '2026-08-17T00:00:00+00:00')
        """
    )
    conn.commit()
    conn.close()

    monkeypatch.setattr(sys, "argv", ["feedback"])
    assert feedback.main() == 0
    conn = sqlite3.connect(str(db_path))
    conn.row_factory = sqlite3.Row
    applied = conn.execute(
        "SELECT COUNT(*) n FROM verifications WHERE applied_at IS NOT NULL"
    ).fetchone()["n"]
    conn.close()
    assert applied == 1

    # Second run: nothing left to fold, exits cleanly without touching add_outcome.
    monkeypatch.setattr(sys, "argv", ["feedback"])
    assert feedback.main() == 0