Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
from api.services.blocks.analyzer.cohort import cohort_prompts as cp
from api.services.blocks.analyzer.cohort.cohort_taxonomy import BehaviorCategory

SCHEMA_VERSION = 1
# 2: added `summary`. Every stored comparison predates the field, and the
# freshness check keys on this, so the bump is what makes them regenerate
# rather than serve a headline-less payload forever.
SCHEMA_VERSION = 2

# A trial whose summary covers less than this share of its own step span is
# reported to the reader rather than averaged over silently.
Expand Down Expand Up @@ -100,6 +103,13 @@ class CohortComparisonOutput(BaseModel):
cohort_success: list[str]
cohort_failure: list[str]
categories: list[CategoryComparison]
# LAST, and the order is load-bearing. This schema is handed to the model
# as `response_format` / `output_schema`, and constrained decoding emits
# fields in schema order -- so a `summary` declared above `categories`
# would be generated before the rows it is supposed to be bound by,
# exactly inverting the prompt's "write summary last" rule and inviting a
# headline the categories do not support.
summary: NonEmptyText


class CohortInput(BaseModel):
Expand Down Expand Up @@ -190,6 +200,16 @@ def to_output(self, raw: str) -> dict:
parsed.model_dump(mode="json"), ci.successful, ci.failing
)
out["dropped"] = dropped
# The headline was written against the categories the model produced,
# and validation runs after it. If a whole category failed citation
# checks and was removed, the summary can name a split the panel no
# longer shows -- an unsourced claim sitting above sourced rows, which
# is the one thing this feature is built not to do. Drop it rather
# than let it describe a comparison that is no longer on screen.
# Observation-level drops leave the category standing, so the theme
# still holds and the summary survives them.
if dropped.get("categories"):
out.pop("summary", None)
# Cohort membership is a fact we already hold, not something to take
# from the model. The UI renders these lengths as "N successful, M
# failing"; leaving the model's lists in place would let a fabricated
Expand Down
24 changes: 23 additions & 1 deletion backend/tests/test_cohort_comparison_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from oddish.blocks.block import BlockParseError

from api.services.blocks.analyzer.cohort.cohort_comparison_block import (
SCHEMA_VERSION,
CohortComparisonBlock,
CohortInput,
)
Expand Down Expand Up @@ -42,6 +43,7 @@ def _raw(evidence, schema_version=99):
"schema_version": schema_version,
"cohort_success": ["t1"],
"cohort_failure": ["t2"],
"summary": "Agents took a test baseline before editing.",
"categories": [
{
"category": "testing_verification",
Expand Down Expand Up @@ -77,10 +79,30 @@ def test_prompt_contains_both_cohorts_and_definitions():
def test_to_output_parses_and_stamps_schema_version():
out = _block().to_output(_raw([GOOD_EVIDENCE]))
# The block owns schema_version; a model-supplied value is overwritten.
assert out["schema_version"] == 1
# Asserted against the constant: pinning the literal made a deliberate
# version bump look like a regression.
assert out["schema_version"] == SCHEMA_VERSION
assert SCHEMA_VERSION != 99
assert out["categories"][0]["category"] == "testing_verification"


def test_summary_survives_a_clean_comparison():
out = _block().to_output(_raw([GOOD_EVIDENCE]))
assert out["dropped"]["categories"] == 0
assert out["summary"]


def test_summary_is_dropped_when_a_category_is():
"""A headline written against categories that validation then removed is
an unsourced claim above sourced rows -- exactly what the citation check
exists to prevent, so it must not outlive them."""
fabricated = {**GOOD_EVIDENCE, "trial_id": "does-not-exist"}
out = _block().to_output(_raw([fabricated]))
assert out["dropped"]["categories"] == 1
assert out["categories"] == []
assert "summary" not in out


def test_to_output_validates_citations_before_the_block_persists():
"""Validation must happen in the transform, not after block.run().

Expand Down
1 change: 1 addition & 0 deletions backend/tests/test_cohort_comparison_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def test_full_output_parses():
schema_version=1,
cohort_success=["t1"],
cohort_failure=["t2"],
summary="Agents took a test baseline before editing.",
categories=[
CategoryComparison(
category=BehaviorCategory.TESTING_VERIFICATION,
Expand Down
26 changes: 11 additions & 15 deletions frontend/src/app/(app)/tasks/[task_id]/task-detail-client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,19 @@ import {
type TaskDetailResource,
} from "@/lib/task-detail-resource";
import type { Task, TaskVersionSummary, Trial } from "@/lib/types";
import { formatRelativeTime, prBadge, taskPrUrl } from "@/lib/utils";
import {
formatRelativeTime,
prBadge,
taskPrUrl,
urlWithSearch,
} from "@/lib/utils";
import {
formatLineRange,
parseLineRange,
type LineRange,
} from "@/lib/line-range";
import { sameFilePath } from "@/lib/file-path";
import { taskHasCancellableWork } from "@/lib/job-status";
import { expandTrialParam, shortTrialParam } from "@/lib/trial-url";
import {
ArrowLeft,
ChevronDown,
Expand Down Expand Up @@ -930,9 +934,7 @@ export function TaskDetailClient({
if (drawerHydratedRef.current || isLoading || !task) return;

const params = new URLSearchParams(window.location.search);
// ?trial= is an index against the task this page already addresses; older
// links spell the whole id out and pass through untouched.
const urlTrialId = expandTrialParam(params.get("trial"), task.id);
const urlTrialId = params.get("trial");
// The version's trials arrive a beat after the task itself
// (selectedVersionId is applied by a later effect), so a trial address
// waits for the version to be selected. Keying on the version — not an
Expand Down Expand Up @@ -976,10 +978,7 @@ export function TaskDetailClient({
// the preserved param instead of leaving it inert forever.
useEffect(() => {
if (!unresolvedTrialParamRef.current) return;
const urlTrialId = expandTrialParam(
new URLSearchParams(window.location.search).get("trial"),
task?.id,
);
const urlTrialId = new URLSearchParams(window.location.search).get("trial");
if (!urlTrialId) {
unresolvedTrialParamRef.current = false;
return;
Expand All @@ -990,7 +989,7 @@ export function TaskDetailClient({
hydrationOpeningRef.current = true;
handleSelectTrial(trial);
}
}, [orderedTrials, handleSelectTrial, task?.id]);
}, [orderedTrials, handleSelectTrial]);

// Closing the drawer retires the task pane address along with the URL
// params the sync effect strips — otherwise reopening would write the
Expand Down Expand Up @@ -1040,10 +1039,7 @@ export function TaskDetailClient({
const next = new URLSearchParams(window.location.search);

if (drawer?.mode === "trial") {
next.set(
"trial",
shortTrialParam(drawer.fallbackTrial.id, drawer.fallbackTrial.task_id),
);
next.set("trial", drawer.fallbackTrial.id);
next.delete("drawer");
} else if (drawer) {
next.set("drawer", "task");
Expand Down Expand Up @@ -1079,7 +1075,7 @@ export function TaskDetailClient({
}

if (next.toString() !== current.toString()) {
const url = `${window.location.pathname}${next.toString() ? `?${next.toString()}` : ""}`;
const url = urlWithSearch(next.toString());
window.history.replaceState(window.history.state, "", url);
}
}, [drawer, taskPaneFile, taskPaneLines]);
Expand Down
85 changes: 66 additions & 19 deletions frontend/src/components/cohort-comparison-section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,26 @@ const CATEGORY_LABELS: Record<string, string> = {
environment_tooling: "Environment and tooling",
};

/** All three headings share the panel's mono-uppercase family (Findings and
* Trial QA use it at task-overview-panel.tsx:613, :654). This section's title
* is deliberately one step louder than those two -- 13px foreground against
* their 11px muted -- because it heads a card of its own rather than a list.
* Below it, category and cohort headings hold 11px and separate by colour. */
const SECTION_HEADING =
"text-foreground font-mono text-[13px] font-semibold tracking-wider uppercase";
const CATEGORY_HEADING =
"text-foreground font-mono text-[11px] font-semibold tracking-wider uppercase";
const COHORT_HEADING =
"font-mono text-[11px] font-semibold tracking-wider uppercase";

/** A trial opens in the task page's drawer via ?trial=<id> — there is no
* /trials/<id> route, and the drawer resolves no step anchor, so none is
* emitted rather than linking somewhere that does not exist.
* /trials/<id> route.
*
* ?tab=trajectory lands on the tab the citation is quoting, and #step-<id>
* is the anchor TrajectoryViewer already resolves (trajectory-viewer.tsx:833
* matches /^#step-(\d+)$/ and scrolls to it), so the link arrives at the
* cited step rather than the top of the run. The first step of the span is
* the anchor: it is where the quoted behaviour starts.
*
* ?version= carries the version id, not the number this endpoint takes: the
* page resolves ?trial= against the selected version's trials alone, so a
Expand All @@ -27,12 +44,27 @@ const CATEGORY_LABELS: Record<string, string> = {
function evidenceHref(
taskId: string,
trialId: string,
stepIds: number[],
taskVersionId?: string,
): string {
const params = new URLSearchParams();
if (taskVersionId) params.set("version", taskVersionId);
// Full id: #1203 reverted the shortened ?trial= form and deleted
// shortTrialParam with it. The sync no longer rewrites this param, so
// there is no rewrite for the fragment to survive either -- but
// urlWithSearch still carries it, because every other drawer sync does
// rebuild the address.
params.set("trial", trialId);
return `/tasks/${encodeURIComponent(taskId)}?${params.toString()}`;
params.set("tab", "trajectory");
const anchor = stepIds.length ? `#step-${Math.min(...stepIds)}` : "";
return `/tasks/${encodeURIComponent(taskId)}?${params.toString()}${anchor}`;
}

/** Discovery labels arrive from the model as identifiers (`subagent_delegation`).
* Render them as words; the prompt asks for prose but a stored label written
* before that rule still has to read properly. */
function discoveryLabel(label: string): string {
return label.replace(/_/g, " ").trim();
}

function stepRange(stepIds: number[]): string {
Expand Down Expand Up @@ -74,7 +106,12 @@ function ObservationList({
{obs.evidence.map((ev, j) => (
<a
key={j}
href={evidenceHref(taskId, ev.trial_id, taskVersionId)}
href={evidenceHref(
taskId,
ev.trial_id,
ev.step_ids,
taskVersionId,
)}
className="text-xs text-muted-foreground underline-offset-4 hover:underline"
>
{/* Only the component + step range carries the link colour. The
Expand Down Expand Up @@ -124,7 +161,7 @@ export function CohortComparisonSection({
if (isLoading) {
return (
<section className="border-border flex flex-col gap-2 border-b p-4">
<h3 className="text-sm font-semibold">Successful vs failing agents</h3>
<h3 className={SECTION_HEADING}>Agent capability analysis</h3>
<p className="text-muted-foreground animate-pulse text-xs">
Analyzing agent behavior across successful and failing runs
<EllipsisDots />
Expand All @@ -136,7 +173,7 @@ export function CohortComparisonSection({
if (error) {
return (
<section className="border-border flex flex-col gap-2 border-b p-4">
<h3 className="text-sm font-semibold">Successful vs failing agents</h3>
<h3 className={SECTION_HEADING}>Agent capability analysis</h3>
<p className="text-muted-foreground text-xs">
Could not build the comparison{typeof error === "number" ? ` (${error})` : ""}.
Reload to try again.
Expand All @@ -150,7 +187,7 @@ export function CohortComparisonSection({
if (!data.categories.length) {
return (
<section className="border-border flex flex-col gap-2 border-b p-4">
<h3 className="text-sm font-semibold">Successful vs failing agents</h3>
<h3 className={SECTION_HEADING}>Agent capability analysis</h3>
<p className="text-muted-foreground text-xs">
No differences held up against the stored trajectories for these{" "}
{data.cohort_success.length} successful and {data.cohort_failure.length}{" "}
Expand All @@ -163,30 +200,38 @@ export function CohortComparisonSection({
return (
<section className="border-border flex flex-col gap-4 border-b p-4">
<div className="flex items-baseline gap-3">
<h3 className="text-sm font-semibold">Successful vs failing agents</h3>
<h3 className={SECTION_HEADING}>Agent capability analysis</h3>
<span className="text-xs text-muted-foreground">
{data.cohort_success.length} successful, {data.cohort_failure.length} failed
{data.cohort_success.length} successful, {data.cohort_failure.length}{" "}
failed trials
</span>
</div>
{data.thin_coverage?.length ? (
<p className="text-xs text-muted-foreground">
{data.thin_coverage.length} trial
{data.thin_coverage.length === 1 ? "" : "s"} in this comparison have
summaries covering under half their run; evidence from them is thin.
</p>
{data.summary ? (
<p className="text-sm text-foreground">{data.summary}</p>
) : null}
{/* No thin-coverage warning. `thin_coverage` divides covered steps by
the trial's FULL step count, but components are built from
drop_inert_steps(trajectory) -- so an agent that pads its run with
empty steps can never score above its non-padded fraction. Measured
on scarf-cargotracker v1: all six flagged trials were gemini (0.10
to 0.196, consistent with its 51-91% empty-step padding) while every
Anthropic trial scored exactly 1.00. It flagged the agent, not the
evidence. Restoring a warning here needs the summariser to persist
its post-filter step count as the denominator. */}
{data.categories.map((cat, i) => (
<div
key={i}
className="border-border bg-background/40 flex flex-col gap-2 rounded-lg border p-3"
>
<h4 className="text-sm font-medium">
<h4 className={CATEGORY_HEADING}>
{CATEGORY_LABELS[cat.category] ?? cat.category}
{cat.label ? `: ${cat.label}` : ""}
{cat.label ? `: ${discoveryLabel(cat.label)}` : ""}
</h4>
<div className="grid gap-6 md:grid-cols-2">
<div className="flex flex-col gap-2">
<span className="text-xs uppercase tracking-wide text-emerald-600 dark:text-emerald-400">
<span
className={`${COHORT_HEADING} text-emerald-600 dark:text-emerald-400`}
>
Successful
</span>
<ObservationList
Expand All @@ -196,7 +241,9 @@ export function CohortComparisonSection({
/>
</div>
<div className="flex flex-col gap-2">
<span className="text-xs uppercase tracking-wide text-red-600 dark:text-red-400">
<span
className={`${COHORT_HEADING} text-red-600 dark:text-red-400`}
>
Failed
</span>
<ObservationList
Expand Down
23 changes: 10 additions & 13 deletions frontend/src/components/experiment-detail-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import { QaCostSuffix } from "@/components/qa-cost-suffix";
import { TagEditor } from "@/components/tag-editor";
import { UnifiedDrawerWrapper } from "@/components/unified-drawer-wrapper";
import { fetcher } from "@/lib/api";
import { prBadge, prNumberFromUrl, taskPrUrl } from "@/lib/utils";
import {
prBadge,
prNumberFromUrl,
taskPrUrl,
urlWithSearch,
} from "@/lib/utils";
import {
formatCostUsd,
formatTokenCount,
Expand Down Expand Up @@ -54,7 +59,6 @@ import {
type LineRange,
} from "@/lib/line-range";
import { sameFilePath } from "@/lib/file-path";
import { expandTrialParam, shortTrialParam } from "@/lib/trial-url";

type DrawerMode = "task" | "trial";

Expand Down Expand Up @@ -1153,10 +1157,7 @@ export function ExperimentDetailView({
if (drawerState?.isOpen) {
next.set("task", drawerState.task.id);
if (drawerState.mode === "trial" && drawerState.trial) {
next.set(
"trial",
shortTrialParam(drawerState.trial.id, drawerState.trial.task_id),
);
next.set("trial", drawerState.trial.id);
} else if (pendingUrlTrialId == null) {
// While a deep-linked trial is still resolving, the drawer is in task
// mode but the ?trial= param must survive for the promotion to keep
Expand Down Expand Up @@ -1190,7 +1191,7 @@ export function ExperimentDetailView({
}

if (next.toString() !== current.toString()) {
const url = `${window.location.pathname}${next.toString() ? `?${next.toString()}` : ""}`;
const url = urlWithSearch(next.toString());
// Keep URL query in sync without triggering app-router navigation work.
window.history.replaceState(window.history.state, "", url);
}
Expand All @@ -1201,8 +1202,8 @@ export function ExperimentDetailView({
hydratedFromUrl.current = true;

const urlTaskId = searchParams.get("task");
const trialParam = searchParams.get("trial");
if (!urlTaskId && !trialParam) return;
const urlTrialId = searchParams.get("trial");
if (!urlTaskId && !urlTrialId) return;

// Fall back to task name so hand-written links like ?task=<name> work;
// the URL-sync effect rewrites the param to the canonical id on open.
Expand All @@ -1211,10 +1212,6 @@ export function ExperimentDetailView({
tasksForExperiment.find((t) => t.name === urlTaskId))
: null;

// ?trial= is an index against the task in the address; older links spell
// the whole id out and pass through untouched.
const urlTrialId = expandTrialParam(trialParam, task?.id ?? urlTaskId);

if (urlTrialId) {
// The trial id is the source of truth for its host task, so scan every
// loaded task rather than trusting ?task= — a stale or missing task
Expand Down
Loading
Loading