Skip to content

Surface the XGBoost conditional baseline in per-question twin tables#73

Merged
johnjosephhorton merged 1 commit into
mainfrom
report-conditional-baseline-column
Jul 9, 2026
Merged

Surface the XGBoost conditional baseline in per-question twin tables#73
johnjosephhorton merged 1 commit into
mainfrom
report-conditional-baseline-column

Conversation

@johnjosephhorton

Copy link
Copy Markdown
Contributor

Problem (from a real research-agent run)

In twin-validation.html, the per-question tables compared the twin only against uniform random and the empirical-marginal oracle — the trivial floors. The XGBoost conditional baseline, which is the deployable bar the twin must beat, was invisible:

  • it was never a comparison column;
  • its rows rendered as a phantom "twin set" row (because it's just another model_label in summary_by_question);
  • and in the report bundle the baseline job was filtered out of scope whenever the caller passed only twin --job-ids, so the comparison couldn't appear at all.

Net effect: the report leaned on "beats uniform" (easy) and buried the one comparison that matters.

Fix

Rendering (render_twin_summary_report_html): the conditional baseline is pulled out of the twin-set loop and gets

  • a "NLL improvement vs conditional baseline" column, placed as the primary comparison;
  • its own reference row, "Conditional baseline (XGBoost) — the deployable bar";
  • a takeaway judged against it: Beats / Edges / Ties / Below conditional baseline.

All gated on the baseline being present — reports without one render byte-for-byte as before. Detection matches the baseline model_label even when the twin-set label is prefixed with "<job_id> / ".

Scoping (report_bundle): folds the conditional-baseline rows for the twin's held-out questions back into the twin report regardless of selected job ids (picking one baseline job if several cover the questions), so the column actually populates. The executive summary stays twin-only for now.

Testing

  • pytest — 254 passed (adds tests/test_report_conditional_baseline.py: column + reference row + baseline-judged takeaway appear when a baseline is present; nothing changes when it isn't).
  • ruff clean.
  • Verified the bundle scoping helper picks the right baseline job and drops off-question / duplicate rows.

Deliberately deferred (follow-ups)

  • The executive summary still compares twin-vs-uniform only — it should also show the conditional baseline (needs the baseline plumbed into build_executive_summary).
  • The model-summary table still lists the baseline as a peer row.
  • The broader report-page overlap (index / executive-summary / twin-validation) is a separate IA cleanup.

🤖 Generated with Claude Code

The per-question performance tables compared the twin only against the trivial
floors (uniform random, empirical-marginal oracle). The XGBoost conditional
baseline — the deployable bar the twin actually has to beat — never appeared as
a column; worse, its rows rendered as a phantom "twin set" row, and in the
report bundle the baseline job was filtered out of scope entirely when the
caller selected only twin job ids, so the comparison was simply missing.

Two fixes:

- Rendering (render_twin_summary_report_html): pull the conditional baseline out
  of the twin-set loop and give it (a) a "NLL improvement vs conditional
  baseline" column as the primary comparison, (b) its own reference row labelled
  "Conditional baseline (XGBoost)", and (c) a takeaway judged against it ("Beats
  / Edges / Ties / Below conditional baseline") instead of against uniform. All
  gated on the baseline actually being present, so no-baseline reports render
  exactly as before. Detection matches the baseline model_label even when the
  twin-set label is prefixed with "<job_id> / ".

- Scoping (report_bundle): fold the conditional-baseline rows for the twin's
  held-out questions back into the twin report regardless of which twin job ids
  were selected, so the column populates. Picks a single baseline job if several
  cover the questions; the executive summary stays twin-only for now.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@johnjosephhorton johnjosephhorton merged commit e2624b0 into main Jul 9, 2026
1 of 3 checks passed
@johnjosephhorton johnjosephhorton deleted the report-conditional-baseline-column branch July 9, 2026 13:24
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR surfaces the XGBoost conditional baseline as the primary per-question comparison in the twin-validation report, fixing a gap where per-question tables only compared twins against trivial floors (uniform random, empirical marginal) and the deployable bar was invisible.

  • twin_report_html.py: Adds a _is_conditional_baseline detector, a new "NLL improvement vs conditional baseline" column, a Conditional baseline (XGBoost) reference row per question, and updates question_takeaway to judge against the baseline when baseline data is present. Column counts are consistent across all row types; signed_cell(None) returns an empty cell preventing misalignment when a question lacks baseline data while show_conditional=True.
  • report_bundle.py: Adds conditional_baseline_rows_for_questions to fold the baseline back into report_rows regardless of selected job IDs, using a Counter-based heuristic to pick the most-covered baseline job when duplicates exist. Twin-only fields continue to use the unaugmented twin_rows.
  • Tests: Two new tests verify column presence, reference-row content, baseline-judged takeaway, and backward-compatibility; the conditional_baseline_rows_for_questions scoping helper is not directly unit-tested.

Confidence Score: 4/5

Safe to merge; the rendering changes are well-isolated and verified correct for all cell-count combinations, and the backward-compat test confirms no regression when baseline data is absent.

The rendering logic in twin_report_html.py is sound with correct column counts and null-safe cell helpers. The one open area is conditional_baseline_rows_for_questions in report_bundle.py, which has no unit test; a wrong job-selection or question-intersection edge case there would silently omit the baseline column in production reports.

zwill/report_bundle.py — the new conditional_baseline_rows_for_questions helper has no unit test; consider adding one to guard the job-selection heuristic.

Important Files Changed

Filename Overview
zwill/twin_report_html.py Adds conditional-baseline detection, column, reference row, and updated takeaway logic; column counts verified correct; takeaway cascade intentionally supersedes trivial-floor verdicts when baseline is present.
zwill/report_bundle.py Adds conditional_baseline_rows_for_questions helper and folds baseline rows into report_rows; scoping logic is correct but the helper is not directly unit-tested.
tests/test_report_conditional_baseline.py New tests cover the rendering path (column presence, reference row, baseline-judged takeaway, and backward-compat without baseline); does not test the report_bundle scoping helper directly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[all_prediction_rows from JSONL] --> B[Filter out BASELINE_MODEL_LABEL exact match to twin_rows]
    B --> C{selected_job_ids?}
    C -- yes --> D[Filter twin_rows by job_id]
    C -- no --> E[Keep all twin_rows]
    D --> F[twin_rows scoped]
    E --> F
    F --> G[conditional_baseline_rows_for_questions]
    G --> H{baseline candidates for heldout questions?}
    H -- no --> I[baseline_rows = empty]
    H -- yes --> J[Pick job with most rows via Counter.most_common]
    J --> K[baseline_rows]
    I --> L[report_rows = twin_rows + baseline_rows]
    K --> L
    L --> M[build_twin_report report_rows]
    M --> N[render_twin_summary_report_html]
    N --> O{show_conditional? any BASELINE_MODEL_LABEL in summary_by_question}
    O -- yes --> P[Add NLL vs conditional column + XGBoost reference row + baseline-judged takeaway]
    O -- no --> Q[Render unchanged no new column]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[all_prediction_rows from JSONL] --> B[Filter out BASELINE_MODEL_LABEL exact match to twin_rows]
    B --> C{selected_job_ids?}
    C -- yes --> D[Filter twin_rows by job_id]
    C -- no --> E[Keep all twin_rows]
    D --> F[twin_rows scoped]
    E --> F
    F --> G[conditional_baseline_rows_for_questions]
    G --> H{baseline candidates for heldout questions?}
    H -- no --> I[baseline_rows = empty]
    H -- yes --> J[Pick job with most rows via Counter.most_common]
    J --> K[baseline_rows]
    I --> L[report_rows = twin_rows + baseline_rows]
    K --> L
    L --> M[build_twin_report report_rows]
    M --> N[render_twin_summary_report_html]
    N --> O{show_conditional? any BASELINE_MODEL_LABEL in summary_by_question}
    O -- yes --> P[Add NLL vs conditional column + XGBoost reference row + baseline-judged takeaway]
    O -- no --> Q[Render unchanged no new column]
Loading

Reviews (1): Last reviewed commit: "Surface the XGBoost conditional baseline..." | Re-trigger Greptile

Comment thread zwill/report_bundle.py
Comment on lines +7 to +28
def conditional_baseline_rows_for_questions(
all_rows: list[dict[str, Any]], twin_rows: list[dict[str, Any]]
) -> list[dict[str, Any]]:
"""Conditional-baseline rows covering the twin's held-out questions.

The baseline is the deployable bar the twin must beat, so it belongs in the
report even when the caller selected only twin job ids. If several baseline
jobs cover these questions, keep the one with the most rows (avoids
double-counting the same model_label).
"""
heldout = {str(row.get("heldout_question")) for row in twin_rows if row.get("heldout_question")}
if not heldout:
return []
candidates = [
row
for row in all_rows
if row.get("model_label") == BASELINE_MODEL_LABEL and str(row.get("heldout_question")) in heldout
]
if not candidates:
return []
by_job = Counter(str(row.get("job_id")) for row in candidates)
best_job = by_job.most_common(1)[0][0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Bundle scoping helper has no unit test

conditional_baseline_rows_for_questions implements the most novel logic in this PR — the job-selection heuristic (pick the baseline job with the most rows) and the held-out-question intersection — but it is exercised only through the manual verification mentioned in the PR description. The new test_report_conditional_baseline.py only hits render_twin_summary_report_html; the build_report_bundle path that calls this helper is not covered. If the heldout_question filtering or the most_common(1) job-picker ever drifts, there's no regression net. A small parametric test against the helper directly (e.g., two baseline jobs with different question coverage, one dominant) would close this gap.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread zwill/twin_report_html.py
Comment on lines +1238 to +1245
if nll_vs_conditional is not None:
if nll_vs_conditional >= 0.05:
return "Beats conditional baseline"
if nll_vs_conditional >= 0.01:
return "Edges conditional baseline"
if nll_vs_conditional <= -0.02:
return "Below conditional baseline"
return "Ties conditional baseline"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Downstream takeaway branches become dead code once nll_vs_conditional is not None

When a conditional baseline is present the function always returns one of the four conditional verdicts and never reaches "Strong individual signal", "Individual signal", "Beats uniform", or "Close to baseline". This is clearly intentional, but it means a twin that ties the conditional baseline (nll_vs_conditional in the range -0.02 to 0.01) while showing very strong empirical-marginal signal will surface as "Ties conditional baseline" and lose the "Strong individual signal" diagnostic entirely. The PR description explicitly defers this as a follow-up, so this is just a flag for the follow-up ticket rather than a blocking concern.

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