feat(eval): behavioral-eval foundation — referee surface + completion relabel (ent#206)

[dolho]

Traceability. Closes Abilityai/trinity-enterprise#267 — children 1 + 2 of the eval epic (trinity-enterprise#205). That issue was filed retroactively: #205's sub-issues covered children 3–7 but never children 1–2, which is why this PR originally had no resolvable reference. Cross-repo closing keywords do not fire the label automation, so status-in-dev needs setting by hand at merge.

Open-core: OSS-core, decided at the gate (trinity-enterprise#206 §10, merged as #215) — not by omission. The table + write fence, the Tier-0 deterministic runner, the agent-owned case runner and the relabel are edition-agnostic primitives; the managed grading experience is the paid layer. Same split as #668.

First implementation off the ent#206 strategy gate (Abilityai/trinity-enterprise#215). The two lowest-dependency children from §11: the referee surface (child 2) and the completion relabel (child 1).

Why this is an OSS PR, not part of the strategy PR (#215). The strategy doc lives in the enterprise submodule; these are OSS primitives (the enforcement layer stays OSS per §10). Separate git repos → separate PRs. Cross-linked.

Referee surface + the write-fence (the load-bearing rule)

A score is only trustworthy if the graded agent cannot write it.

agent_reports (#918) was disqualified as the surface — verified self-gated, so an agent writes its own report. This surface inverts that:

• agent_evaluations table — a run's quality grade, kept apart from the clean-exit completion mirror. Dual-track (feat(db): adopt Alembic for PostgreSQL migrations (retire SQLite-only bespoke runner) #1183): schema.py + migrations.py (SQLite) and Alembic 0029 (PostgreSQL); AGENT_REFS CASCADE.
• db/evaluations.py (three-layer) — the only writers are the platform and a future evaluator; no agent-scoped key reaches it.
• routers/evaluations.py — write is human-admin-only (require_admin + reject_agent_principal, so an agent key that inherits its owner's role still can't write — the trinity-ops-agent#232 trap). Read is access-scoped: an agent reads its own evaluations (feedback ✓), an admin reads all. Read ≠ write.

Verified live on the running instance:

admin write                      -> 200  (eval_82666de8…, quality 0.9)
graded agent's own key, WRITE    -> 403  ← the fence
graded agent's own key, READ     -> 200  ← feedback allowed

Completion relabel (the honesty fix)

status='success' is a clean process exit, rendered as "Success rate" on Overview (#1107), the schedules rollup (#1115) and fleet stats (EXEC-022) as if it were correctness. Relabelled those three surfaces to "Completion" with a tooltip ("finished without erroring — not answer quality"). Additive — the success_rate API field is unchanged (back-compat); only the user-facing labels move. The quality axis now lives on the referee surface, populated by the Tier-0 runner (child 3, next PR).

Tests

tests/unit/test_206_agent_evaluations.py — 10 passed: independent completion/quality axes, nullable quality, agent-scoped + accessible-set reads, and the fence (write route rejects an agent principal, is admin-gated, no agent-writable route exists, and agent_reports is not reused). The schema-parity guard caught a missing-index gap in schema.py (indexes in the migration but not the fresh-install path) — fixed. 128 passed across schema/migration/parity suites.

What's NOT here (next children, per §11)

• Child 3: the Tier-0 deterministic evaluator that populates quality (report-landed, changed-vs-last, budget, trajectory shape). This PR is the surface it writes to.
• Child 4: the agent-owned case runner.
• Child 5 (ENT): the Tier-1 LLM judge.
• Child 7 (gated on feat: effect-scoped idempotency keys for outbound side effects #1084/feat: deterministic tool-side gate on capability-confined irreversible rails (v2 gap) #1408): replay/shadow.

🤖 Generated with Claude Code

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[AndriiPasternak31]

Validated via /validate-pr. Requesting changes — two required surfaces are missing outright, plus conflicts.

1. No documentation at all

Zero files under docs/memory/ are touched. This PR adds:

• a new table — agent_evaluations (db/schema.py, db/tables.py, db/migrations.py, Alembic 0031)
• a new db module — src/backend/db/evaluations.py
• a new router — src/backend/routers/evaluations.py, registered in main.py
• new models in models.py
• changes to three frontend components (ExecutionsPanel.vue, OverviewPanel.vue, ScheduleAnalyticsCard.vue)

Under the tiered rule that's a new capability, which owes docs/memory/requirements/<area>.md plus a feature flow; the new router/endpoints and the new table separately owe an architecture.md entry (router catalog + Database Schema). Rule 1 also puts the requirements update before implementation, so this is the write-path being skipped rather than a tail step that got dropped.

2. No entitlement gating

The diff has zero occurrences of requires_entitlement, register_module, or enterprise_features. Per the SDLC section in CLAUDE.md, a feature filed on the enterprise tracker is treated as a gated module by default, and "it reuses OSS tables/endpoints" explicitly doesn't imply OSS-core — that's a product call, not an implementation default, and the guidance is to confirm the gating shape before building. As it stands this ships an ungated OSS-core surface by omission rather than by decision.

If OSS-core is the intended call, that's fine — but it needs to be stated on the issue, and the enforcement primitive still belongs on the OSS side with the feature logic in the private submodule. Either way the shape needs confirming before this lands, because unwinding a shipped-ungated surface later is much more expensive than gating it now.

3. Traceability

ent#206 isn't a reference form GitHub resolves, and no open implementation issue is linked. The reference needs the fully-qualified closing-keyword form, pointed at an open issue — and since cross-repo closing keywords don't fire the automation, someone needs to set status-in-dev manually at merge.

4. Conflicting with dev

Five core files conflict: database.py, db/migrations.py, db/schema.py, db/tables.py, main.py. Because it's conflicting there's no refs/pull/N/merge, so no CI is reporting on this PR at all — including schema-parity and pg-migrations, which are the two jobs you'd most want on a schema change.

What holds up

The dual-track migration is done correctly and completely — SQLite entry in db/migrations.py, the DDL in both schema.py and tables.py, and Alembic 0031_agent_evaluations chaining cleanly off the current head 0030_slack_channel_allow_proactive. That's all four surfaces, which is more than most schema PRs get right first time. Heads-up: #1628 is also holding an unmerged revision, so whichever of the two lands second will need to re-check the head rather than assume its number is still free.

tests/unit/test_206_agent_evaluations.py is present, and db/agent_cleanup.py is updated — good catch registering the new table for the cascade/rename path rather than leaving it as an orphan for canary L-03 to find later.

Happy to re-review once the docs and the gating decision are in.

[dolho]

All four addressed. Thanks — the docs gap and the traceability gap were both real, and the conflict note was the one that mattered most, since it meant nobody was going to see CI on a schema change.

1. Documentation — added

• requirements/lifecycle-observability.md §46 — the two axes (completion vs quality), FR-1..FR-5, and the reasoning that decides the design: agent_reports (§44) was evaluated and rejected as the surface because its create is self-gated by design, which is precisely the wrong boundary for a grade.
• feature-flows/agent-evaluations.md (new) — flow diagram, the write fence and why it needs both gates, the nullable-quality contract (null = "not graded yet", not 0 — the axes are independent), and what this deliberately does not include.
• architecture.md — router catalog entry next to reports.py, a subsystem block, and the agent_evaluations DDL in Database Schema.
• feature-flows.md — changelog row + index row.

You're right that Rule 1 makes this a write-path miss rather than a dropped tail step. Noted.

2. Entitlement gating — OSS-core, and it was decided at the gate

This one I'd push back on, with evidence rather than assertion. §10 of the strategy doc (trinity-enterprise#206, merged as #215 — the gate this PR is the first implementation off) makes the call explicitly:

OSS (edition-agnostic primitives):

• agent_evaluations table + the write-fence auth boundary (§4) — the load-bearing rule must be enforceable in every edition.
• The Tier-0 deterministic runner … cheap, always-on, no external calls.
• The completion relabel (§3) — an honesty fix to OSS surfaces.

Enterprise (managed knobs / hosted grading): Tier-1 judge panels, calibration, rubric management UI …

The split mirrors #668 exactly: STATIC (deterministic) is OSS and free; the AI/managed layer is the paid tier.

So this ships ungated by decision, not by omission — and the decision is the one CLAUDE.md asks for: the enforcement primitive stays OSS, the managed layer is the paid tier. The reason it has to be OSS is the rule itself: a score is only trustworthy if the graded agent cannot write it is worth nothing if it holds only in the paid edition.

I've recorded it at the top of the PR body rather than leaving it implicit in a linked doc, since your point stands that "unwinding a shipped-ungated surface later is expensive" — it should be legible at review time. I also verified the enterprise-docs guard pattern locally against the new docs: clean (the split is described the way §45 describes ent#184's, without naming private schema).

3. Traceability — fixed

Closes Abilityai/trinity-enterprise#267, added to the PR body.

That issue is new: epic #205's sub-issues covered children 3–7 (#207 case runner, #208 trajectory, #209 replay) but children 1–2 — the relabel and the referee surface, i.e. exactly what this PR ships — were never filed. So there was no open issue to point at. It's now filed, scoped to the two children, and linked as a sub-issue of #205.

And noted on the cross-repo automation: status-in-dev needs setting by hand at merge — that's called out in the PR body so whoever merges sees it.

4. Conflicts — resolved, merged clean

origin/dev merged in. All five were "both sides added an adjacent entry" (this branch's eval table/router vs dev's ent#184 product_events), so both survive — dev's entry first, ours second, which keeps the migration list in landing order.

Two of them needed care rather than a mechanical take-both: in schema.py and tables.py the two entries shared a trailing ) block, so a naive resolution silently produced an unterminated string in one and an unclosed Table( in the other. Both caught by an AST parse before commit, fixed in cbaceb2e.

mergeable is now true, so CI can finally report — including the two jobs you flagged.

Schema-critical suites, locally:

pytest tests/unit/test_206_agent_evaluations.py tests/unit/test_schema_parity.py \
       tests/unit/test_alembic_parity_guard.py tests/unit/test_agent_cleanup_parity.py \
       tests/unit/test_schema_postgres.py
42 passed, 1 skipped

Alembic chain re-checked after the merge: 0031_agent_evaluations still chains off 0030_slack_channel_allow_proactive, which is dev's current head, and dev's own 0029_product_events slots in cleanly below it. Your heads-up about #1628 holding an unmerged revision still stands for whichever of us lands second — flagging it there too.

Full unit suite result to follow in the next comment.

[dolho]

Full unit suite on the branch after the dev merge:

5049 passed, 16 skipped, 4 failed  (26:35)

FAILED tests/unit/test_1472_schedule_validation.py::test_accepts_valid[0 4 * * *-Europe/Kiev]
FAILED tests/unit/test_admin_email_login.py::test_admin_logs_in_with_username
FAILED tests/unit/test_admin_email_login.py::test_admin_logs_in_with_registered_email
FAILED tests/unit/test_admin_email_login.py::test_email_identifier_is_normalized

None from this branch — the same four this environment shows on an untouched checkout: a missing tzdata module for the first, and a bcrypt has no attribute '__about__' version mismatch (also visible in the backend boot log) behind the other three. Identical set on #1843/#1845/#1847.

The eval-specific and schema-critical suites are green: test_206_agent_evaluations + test_schema_parity + test_alembic_parity_guard + test_agent_cleanup_parity + test_schema_postgres → 42 passed, 1 skipped.

[AndriiPasternak31]

/validate-pr — Validation Report

PR: #1752 · Branch: feature/206-eval-foundation → dev · 18 files (+785/−5) · MERGEABLE · all 23 CI checks green

Summary

Category	Status	Notes
Commit Messages	✅	5 commits, conventional prefixes (feat/fix/docs), descriptive
Base Branch	✅	targets dev
PR Size	✅	18 files
Status Auto-Promotion	⚠️	Closes Abilityai/trinity-enterprise#267 — valid qualified cross-tracker ref, but the automation is same-repo → manual status-in-dev at merge. PR body already flags this.
Roadmap / Issue	✅	ent#267 resolves: priority-p2, type-feature, theme-reliability, complexity-medium, status-in-progress
Requirements	✅	requirements/lifecycle-observability.md §46, FR-1…FR-5 + open-core rationale — but see doc defect below
Architecture	✅	router catalog entry, subsystem block, agent_evaluations DDL
Feature Flows	✅	new feature-flows/agent-evaluations.md + index row
Feature Flow Format	✅	Overview/flow/fence/data-layer/relabel present; lighter on Error Handling + Testing sections than the template
Security Check	✅	no keys, emails, IPs, .env, secrets, credential files; gitleaks green
Infrastructure	➖	no compose/Dockerfile/nginx changes
Build Packaging	✅	no new top-level src/backend/*.py; new files land under db//routers//migrations/ (COPYed wholesale)
Config Packaging	➖	no new os.getenv()
Test Adequacy	✅	10 tests, non-happy-path heavy (fence, tri-state accessible set, nullable quality, cascade)
Code Quality	⚠️	relabel scope incomplete; stale doc refs
Requirements Trace	✅	traces to ent#206 §10/§11 strategy gate

Verified by execution (not by reading the description)

tests/unit/test_206_agent_evaluations.py                                 10 passed
test_schema_parity + test_alembic_parity_guard + test_agent_cleanup_parity
  + test_models_centralized + test_alembic_revision_id_length             70 passed
test_186_enumeration_uniformity + test_1310_auth_wiring
  + test_1310_auth_consolidation                                          80 passed
scripts/ci/check_alembic_parity.py <base> HEAD                            PASS
alembic chain: 0031 → 0030 → … linear, single head, no duplicate down_revision

The write fence is real. Read against dependencies.py:486 (require_admin → _reject_connector_principal + role check) and :420 (reject_agent_principal → 403 when current_user.agent_name is set). An agent-scoped key resolves to its owner and inherits the owner's role, so on a default admin-owned install require_admin alone genuinely would let a graded agent write its own grade. The second gate closes it. create_evaluation is the only writer, and test_there_is_no_agent_writable_evaluation_route scans every function in the module for a future one — a guard that will actually hold.

Issues Found

Critical (block merge)

None.

Warnings (review required)

1. The completion relabel is materially incomplete — including inside its own stated scope.

The PR relabels 3 spots. Five more user-facing surfaces still render the same exit-based success_rate as "Success":

File:line	Label	Scope
OverviewPanel.vue:419	success	Same file, same tab this PR edited — the "Schedules performance" section (#1115)
SchedulesPanel.vue:383	7d success	The other #1115 half — architecture.md names it explicitly ("Backs both the Overview 'Schedules performance' section and the Schedules-tab inline stats")
TasksPanel.vue:76	Success Rate	Computed from e.status === 'success' — literally the conflation this PR exists to fix
AgentTile.vue:104	Success	Grid dashboard tile, fed by network.js:1125 successRate: stat.success_rate
ReplayTimeline.vue:192,216	Success rate	Timeline dashboard (still the default mode)

The PR body claims the #1115 rollup is covered; both of its halves still say "success". The result is mixed vocabulary on a single screen — Overview now reads "Execution completion rate" in one card and "success" in the card directly beneath it, which is arguably worse for coherence than either consistent state.

(ScheduleAnalyticsCard.vue:107 and ExecutionsPanel.vue:109 are correctly left alone — a chart legend and a filter option for the literal status enum value, not rate labels.)

2. Requirements doc asserts canary coverage that does not exist.

requirements/lifecycle-observability.md FR-1 states agent_evaluations is "registered in AGENT_REFS so rename re-keys and purge cascades, and canary L-03's orphan scan covers it." The first half is true and CI-guarded. The second is false: canary/snapshot.py drives L-03 from its own hardcoded ORPHAN_SCAN_TABLES list (:172) with a hardcoded db.tables import block (:749) — it does not derive from AGENT_REFS, and agent_evaluations appears in neither. Either add the two entries or drop the claim.

Suggestions (non-blocking)

• Alembic omits the FK the SQLite track has. schema.py/migrations.py declare FOREIGN KEY (execution_id) REFERENCES schedule_executions(id); 0031_agent_evaluations.py does not. SQLite's is inert (PRAGMA foreign_keys off platform-wide) but PostgreSQL would actually enforce one — a real behavioral difference between tracks, and the parity guard only checks that a revision exists, never column-level shape. If deliberate (a real FK would block grading a purged execution), say so in the revision docstring.
• No retention sweep. agent_evaluations is absent from RETENTION_OPS_KEYS and cleanup_service. Harmless today (admin-only writes), but child 3's Tier-0 evaluator writes a row per execution — unbounded growth. agent_reports, the sibling this surface is modelled on, has agent_reports_retention_days.
• No payload cap on checks/judge. agent_reports caps at 256 KB → 413. Same exposure once child 3 writes checks_json per run.
• Write route doesn't validate the agent exists — agent_name: str is a raw path param under require_admin, so an admin typo mints an orphan row no cascade will reap (#1445 class). Low impact (admin-only, no token/scheduler side effect), but db.is_agent_live() is one line.
• Stale 0029 reference in db/evaluations.py:13 (and the PR body) — leftover from the rebase; the revision is 0031.
• Invariant #1 satisfied by letter, not shape — no services/evaluation_service.py; the router calls db.* directly and FR-5 redefines the third layer as the database.py facade. Fine for pure CRUD and consistent with reports.py read paths, but the module docstring's "Three-layer (Invariant #1)" overstates it.
• Read breadth is wider than documented. Docs say AuthorizedAgentByName gates read to "owner/admin/agent-self"; in fact an agent-scoped key inherits its owner's full accessible set, so it can read a sibling agent's evaluations. Deliberate under "read ≠ write" and identical to agent_reports, but the phrasing implies a self-scope that isn't there.

Recommendation

APPROVE with follow-ups.

The load-bearing part is correct and verified by execution rather than description: the write fence is two genuinely independent gates closing a real inherited-role hole, the surface has exactly one writer, a structural test prevents a second appearing, dual-track migrations are consistent and land on a clean single Alembic head, and AGENT_REFS registration is CI-enforced. Foundation PRs earn their keep by making the next four children safe, and this one does.

Neither warning is a defect in shipped behavior. Warning 2 is a one-line doc fix worth correcting now. Warning 1 is the judgment call: the relabel is additive and safe, but leaving "success" and "Completion" side by side on the same tab means the feature's stated purpose isn't achieved. Finishing the remaining five labels is small and mechanical and probably better here than filed — but sweeping them in child 3 is defensible provided a follow-up issue exists so it doesn't strand.

Reminder for the merger: cross-tracker closing keyword → set status-in-dev on Abilityai/trinity-enterprise#267 by hand.