Skip to content

Ship the leaderboard as columns, and fetch a card's groups when it opens - #380

Merged
ahernsean merged 11 commits into
mainfrom
claude/leaderboard-level-of-detail
Oct 3, 2026
Merged

ahernsean merged 11 commits into
mainfrom
claude/leaderboard-level-of-detail

Conversation

@ahernsean

@ahernsean ahernsean commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

The leaderboard shipped every row's full response-group breakdown. At the full
candidate vocabulary that projects to 61.6 MB — a payload no phone renders
past, and the one virtualization cannot help with, because the browser
downloads and parses every byte before the first row draws.

Measured against the production cache and projected to 14,855 openers:

at 14,855
main 61.6 MB
this branch 0.29 MB
212x smaller

One opener's breakdown is 5.9 KB, fetched when a card is opened.

The ranking travels as columns

A row-shaped ranking is mostly spelling at this scale. Measured per field, with
response_groups already gone:

answer_count           19 B/row   0.28 MB   <- constant: every opener partitions all 3,209
erd                    23 B/row   0.35 MB
max_remaining_depth    23 B/row   0.34 MB
rank                   10 B/row   0.15 MB   <- implicit in sort order
word                   14 B/row   0.21 MB
word_is_answer         22 B/row   0.32 MB   <- 2 distinct values

JSON keys are 63% of a lean row, and half the payload is constants or
derivable. So the ranking ships as parallel arrays: words as one fixed-width
string, erd_numerator and max_remaining_depth as arrays, word_is_answer
as a base64 bitset in the shape PR #365 established, answer_count hoisted,
rank left implicit. Lean objects would have been 1.75 MB; columns are 0.29 MB.

ERD is carried exactly, and there is no decimal beside it

An opener's ERD is total guesses over the answer count, so the numerator counts
guesses and the value is on its branch's lattice by construction. Verified
across all 901 live rows: zero off-lattice, worst deviation from an integer
3.3e-11.

The exact form is smaller than the approximate one — 11412 against
3.556248052352757 — so the cards gain exact fractions while the payload
shrinks. This delivers the leaderboard's share of #307.

An off-lattice value is therefore not a display case to fall back from: it
means the fold produced something an ERD cannot be, and every other value in
the ranking is suspect with it. The ranking raises rather than quietly showing
a decimal, naming the opener and the value.

erd_lattice_numerator still returns None, because what None means
differs by quantity — a derived ceiling carries epsilon padding and can
legitimately sit between lattice points, so None there is ordinary. The
caller decides.

An unchanged ranking costs nothing

The client already skipped the redraw when the rows were unchanged, but still
fetched, parsed and diffed the whole body every two seconds — while the server
already held the answer in the opener_completion_signal it revalidates
against, and never told anyone.

The server now hands out an ETag. The client sends it back as
If-None-Match, the server answers 304 with no body, and the client
returns without parsing or comparing. An opener completes about every 27
minutes against a poll every two seconds, so roughly 810 of every 811 polls
now transfer nothing
.

The tag is a blake2b of the encoded body, not of the completion signal the
server rebuilds on. Those answer different questions. The signal asks whether a
rebuild is worth doing, and is deliberately approximate — a repair or an import
moves the cache without completing any queue work, which is exactly what
REPORT_CACHE_MAX_AGE_SECONDS is a backstop for. The client asks whether what
it holds is still current, and that admits no approximation: a wrong answer is
a stale render with no path to recovery. Hashing the representation answers the
client's question by construction, and costs one hash per build (377 µs on a
0.24 MB body at 14,855 rows) rather than anything per poll.

If-None-Match is sent only for the view already on screen. A 304 carries no
body, and the client keeps one rendered report at a time — deliberately leaving
the previous view up while the next is fetched — so revalidating after
navigating away would answer 304 for the view being entered and leave the
other one on screen under its tab.

The fetch needs cache: "no-store": without it the browser satisfies the
revalidation itself and hands back a synthesised 200, hiding the 304 from the
client and reinstating the parse — a change that would measure as working at
the network layer while changing nothing above it.

A render an interaction blocked is owed, not lost

A poll never replaces the report while a control inside it holds focus or a
text selection crosses it, because replaceChildren would delete the thing
under the reader's finger. Opening a card puts focus on its <summary>, so
this branch, which the tree view had reached rarely, is now on the leaderboard's
ordinary path.

The suppressed report was still banked as lastReport and its tag still
stored — correctly, since both describe what the server holds. But nothing
then noticed that the screen was a report behind: the next poll revalidated
against the new tag and got 304, or compared the same body against the banked
one and found it unchanged. Either way the ranking the reader was waiting for
never arrived, and with the swarm stopped it never would.

The debt is now recorded and settled by the first poll that finds the
interaction over, redrawing from the report the client already holds. It is
paid on the 304 path too, which is the ordinary one: the ranking stops moving
the moment a reader opens a card.

Cards expand progressively

A collapsed card is rank, word, ERD and worst case — 14 DOM nodes, against
the full breakdown's 44 on a five-group fixture. At 14,855 that is 208,000
nodes against a projected 1,640,000. Still far too many to render as a flat
list, which is what #371 is for; this unblocks it rather than replacing it.

Opening a card fetches its own breakdown. The open set and the fetched
breakdowns live outside the DOM, following the tree's collapsedNodes idiom,
because the client re-renders on every poll and anything held in the DOM would
be destroyed twice a second.

The screen says when its data is from

A report served from revalidation is not refreshed by the poll — its data is as
old as the last time its signal moved. Every other view is rebuilt on each
two-second poll, so for those the cadence is the freshness; for this one it
is not, and nothing on screen said so.

The server marks such a report revalidated, and the client shows
data as of 22 Sep 2026, 05:41:03 beside it, following the snapshot line the
tree already carries and using the existing en-GB helper. The mark is set
where the revalidation decision is made rather than by listing the kinds again
in JavaScript, where the two could drift.

Tests

leaderboard_rows() is the one place that reads the column encoding; the
terminal renderer goes through it and its output is unchanged.

  • LEADERBOARD_COLUMNS_JS builds columnar payloads for the browser tests, in
    the shape SWEEP_BITMAP_JS established.
  • _open_leaderboard_breakdown() opens a card for every assertion about
    segments, legends or group menus.
  • leaderboard-word.json and its routing follow the openers-word.json
    precedent, so the fixture server can serve a named opener's detail.
  • The ranking test now asserts the groups are absent from a collapsed card.
  • The leaderboard fixture had been built off-lattice on purpose to "cover
    both paths", which tested a shape production cannot produce. Both rows now
    sit on the lattice, as every exact ERD does.
  • A revalidated report carries the mark and a live one does not — the queue
    report is deliberately excluded from revalidation because its subject is
    what the swarm is doing right now.
  • Conditional polling is covered on both sides: three server tests (tag
    present, 304 with empty body, a rebuild the signal cannot see is not
    withheld) and browser tests for the tag being sent back, a 304 leaving the
    cards alone, and a ranking deferred behind an open card being drawn once the
    card is let go — on a 304 and on an unchanged body alike.

Closes #367. Refs #307.

The ranking carried every row's full response-group breakdown, which projects
to 61.6 MB at the full candidate vocabulary -- the wall virtualization cannot
help with, because the browser parses every byte before the first row draws.
Measured on the production cache and projected to 14,855 openers, this takes
the payload to 0.29 MB.

A row-shaped ranking is mostly spelling at that scale: JSON keys are 63% of a
row once its groups are gone, `answer_count` is the same number in all 14,855
of them, and rank is the array index.  So the ranking travels as parallel
arrays -- `words` as one fixed-width string, `word_is_answer` as a base64
bitset in the shape PR #365 established, the constant hoisted.  Lean objects
would have been 1.75 MB.

ERD is carried exactly, and that made it smaller rather than larger.  An
opener's ERD is the mean line length over the answer list, so it is
numerator/answer_count for an integer numerator; across all 891 live rows the
worst deviation from an integer is 3.3e-11.  `11412` replaces
`3.556248052352757` -- more precise and twelve characters shorter -- so the
cards gain exact fractions while the payload shrinks.  erd_lattice_numerator
is the Python-side helper #307 asks for, and a value off the lattice keeps its
decimal rather than being snapped.

A collapsed card is 14 DOM nodes and fetches its own breakdown when opened.
The open set and the fetched breakdowns live outside the DOM, following the
tree's collapsedNodes idiom, because the report re-renders on every poll and
anything held in the DOM would be destroyed twice a second.

leaderboard_rows() is the one place that reads the encoding; report_terminal
goes through it with unchanged output.  The leaderboard fixture previously
held only off-lattice ERDs, so the exact path was untested -- it now covers
both, and the ranking test asserts the groups are absent from a collapsed card.

Closes #367.  Refs #307.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2000a0c05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread report_terminal.py
ahernsean and others added 2 commits September 22, 2026 01:09
Two things the first pass got wrong.

**An exact ERD cannot be off its branch's lattice.**  It is total guesses over
the answer count, so the numerator counts guesses and is an integer by
construction.  Carrying an `erd_decimal` beside it modelled a state that cannot
occur -- across 901 live rows the column was empty in every payload -- and the
fixture had been built off-lattice on purpose to "cover both paths", which
tested a shape production never produces.

The column is gone.  A value that does not land on the lattice now raises,
naming the opener and the value, because it means the fold produced something
an ERD cannot be and every other value in the ranking is suspect with it.
`erd_lattice_numerator` keeps returning None, because what None *means* differs
by quantity: a derived ceiling carries epsilon padding and can legitimately sit
between lattice points, so None there is ordinary.  The caller decides.

**An unchanged ranking should cost nothing.**  The client already skipped the
redraw, but still fetched, parsed and diffed the whole body every two seconds --
while the server already held the answer in the opener-completion signal it
revalidates against, and never told anyone.

That signal is now the ETag.  The client sends it back, the server answers 304
with no body, and the client returns without parsing or comparing.  An opener
completes about every 27 minutes against a poll every two seconds, so this is
the ordinary poll rather than an optimisation for a rare one.

The fetch needs cache:"no-store": without it the browser satisfies the
revalidation itself and hands back a synthesised 200, hiding the 304 from the
client and reinstating the parse -- a change that would measure as working at
the network layer while changing nothing above it.

Seven new tests, each run against the change reverted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
CI caught a leaderboard read the terminal renderer still made directly.  The
scripted edit had two sites and matched one; the check that was supposed to
catch that asserted the replacement text appeared *somewhere* in the file,
which the first site had already made true.  The assertion is now scoped to the
function's own body, so the hotspots renderer's legitimate rows read cannot
satisfy it.  tests/test_report_pipeline.py is the module that renders this and
was the one outside my local run.

A report served from revalidation is not refreshed by the poll: its data is as
old as the last time its signal moved -- up to REPORT_CACHE_MAX_AGE_SECONDS,
and in practice as old as the last opener to finish.  Every other view is
rebuilt on each two-second poll, so for those the cadence is the freshness; for
this one it is not, and a reader has no way to tell them apart.

The server marks such a report `revalidated`, and the client shows
"data as of <time>" beside it, following the snapshot line the tree already
carries.  The mark is set where the revalidation decision is made rather than
by listing the kinds again in JavaScript, where the two could drift.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PpacbZRqT46b4Duhxq5QNQ
@ahernsean

Copy link
Copy Markdown
Owner Author

@codex, another review, please

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2422c276a9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread report_server.py Outdated
The ETag was taken from opener_completion_signal -- the value the server uses
to decide whether a rebuild is worth doing.  That signal is deliberately not
exhaustive: a repair, a reverification or an import changes the cache while
completing no queue work, which is exactly what REPORT_CACHE_MAX_AGE_SECONDS
exists to catch by rebuilding once the entry ages out.

A validator taken from the signal goes on matching across that rebuild, so the
server answers 304 while holding a ranking it has already replaced, and the
client keeps the stale one until some unrelated opener completes -- with the
queue stopped, never.  The backstop fires and accomplishes nothing.

The two are different questions asked at different moments.  The server asks
whether a rebuild is worth doing, and a proxy that occasionally misses is fine
there because missing costs freshness and never correctness.  The client asks
whether what it holds is still current, which admits no approximation at all.

body_validator hashes the representation, so it answers the client's question
by construction.  It also strictly dominates: a rebuild producing identical
bytes now revalidates for free, which the signal-derived tag could not do --
an opener completing below the visible rows used to force a pointless full
send.  The tag is stored beside the body it names, so it costs one hash per
build (377 us at 14,855 rows) and nothing per poll.

Reverting to the signal-derived tag fails two of the three tests by name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
@ahernsean

Copy link
Copy Markdown
Owner Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66b4295909

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread report_client.html
Comment thread report_client.html Outdated
Opening a card asked for the leaderboard with a branch_target, and the report
built the whole ranking before appending the detail: measured at 2.47s a click,
returning 13 KB of columns the client already held -- 0.29 MB of them at the
full vocabulary.  Worse, the request is its own cache key, so a reader who
opened more cards than REPORT_CACHE_MAX_ENTRIES pushed out the ranking they
were polling and made the next poll rebuild it.  Expansion cost more than the
ranking it was meant to spare.

A named opener now stops before the vocabulary screen.  Its groups are one
candidate's partition against the answer list, and its fold reads the states of
its own groups through report_branch_states rather than loading every reusable
branch fact in the cache -- _screen_and_fold_openers reads 652,989 rows because
it screens 14,855 candidates, and a card needs 158.  **2,470 ms to 15 ms**, and
the response carries no columns at all.

Such a request also stays out of the revalidated cache.  It builds in
milliseconds, so there is nothing to save, and the cache holds eight entries
against a vocabulary of words.

An unavailable breakdown is no longer kept.  "Not finished yet" describes the
instant, not the opener -- the sweep is still running, or a repair has just
invalidated it -- so keeping it answered every later expansion with it, and a
card opened early read as unfinished for the life of the page.  A ranking whose
validator has moved also drops the breakdowns fetched against the previous one,
since a new ranking is a new set of facts about every opener in it.

Six tests, each run against its change reverted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
Comment thread report_client.html Outdated
Comment on lines +2672 to +2682
const requestHeaders={Accept:"application/json"},cachedTag=reportEntityTags.get(context);
if(cachedTag)requestHeaders["If-None-Match"]=cachedTag;
const response=await fetch(buildAPIURL(state),{headers:requestHeaders,cache:"no-store",signal:controller.signal});
if(generation!==requestGeneration)return;
if(response.status===304){lastSuccess=Date.now();failureCount=0;connection(true,"connected");return;}
const entityTag=response.headers.get("ETag");
// A new ranking is a new set of facts about every opener in it, so the
// breakdowns fetched against the previous one are no longer known to
// hold. Dropping them costs one fetch per card still open.
if(entityTag&&reportEntityTags.get(context)!==entityTag)openerBreakdowns.clear();
if(entityTag)reportEntityTags.set(context,entityTag);else reportEntityTags.delete(context);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: a 304 can leave a different view's report on screen after navigating away and back.

reportEntityTags is keyed by contextKey(state) (== buildAPIURL(state)) and is never cleared on navigation. The client keeps only one rendered report at a time (lastReport/lastContext), and switching views deliberately leaves the previous view's DOM in #report while the new fetch is pending (see the comment two lines above this hunk, "Keep the current report visible while the replacement is being computed").

Sequence: view Leaderboard → 200, tag stored, rendered → switch to another view (e.g. Queue) → renders instead, lastContext becomes the queue context → switch back to Leaderboard within REPORT_CACHE_MAX_AGE_SECONDS (120s) → same context key is requested, If-None-Match matches the server's cached tag → 304 → line 2676 returns immediately with no applyReport call, no lastContext update. The Queue report stays on screen under the Leaderboard tab (which syncControls has already marked active) for up to ~2 minutes, or until the cached body's bytes actually change.

test_a_304_leaderboard_poll_leaves_the_rendered_cards_alone only covers the same-view case, so it doesn't catch this.

Suggested fix: only send If-None-Match when context===lastContext (the steady-state poll is exactly that case, so no benefit is lost), or retain a per-context rendered report to redisplay on 304.

Comment thread report_model.py
Comment on lines +3628 to +3637
if detail_word:
# One opener's groups, and nothing else. Screening the whole
# vocabulary to answer a question about a single card would cost
# more than the ranking that card came from, and would send the
# columns back to a client already holding them.
data["detail"] = _one_opener_detail(
sources, cache, all_answers, all_candidates, detail_word,
group_budget, answer_set)
report["sources"]["cache"]["ok"] = True
return report

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: the single-opener detail response is missing keys the terminal renderer requires unconditionally, causing a KeyError crash.

This early return only ever sets data["response_pattern_count"] and data["detail"] — counts, candidate_count, and total_rows are only added in the full-ranking path further down (the data.update({...}) block). But report_terminal.py's _render_leaderboard_sections does, unconditionally:

counts = data["counts"]          # report_terminal.py:1822
...
f"candidates={data['candidate_count']}"
... f"(showing {len(leaderboard_rows(data))} of {data['total_rows']})"  # https://github.com/ahernsean/wordle/blob/16d4d5e45a8f29b79a0d423c6f07690f558c391b/report_terminal.py#L1833-L1835

with no "detail" in data guard and no detail-rendering branch.

erd_search.py view --leaderboard <WORD> reaches exactly this: view's positional spine argument is accepted alongside --leaderboard with no guard rejecting the combination (unlike the existing openers/root_progress branch-target checks in validate_report_request), parse_report_branch_target("crane") produces a bare-word branch_target with no steps, collect_leaderboard_report takes this early-return branch, and the default text renderer crashes with KeyError: 'counts'. (--json/--jsonl are unaffected since they serialize the dict directly.)

No existing test exercises this combination — tests/test_report_pipeline.py's leaderboard test uses the default root target, and tests/test_report_terminal.py's leaderboard fixtures always supply counts.

Suggested fix: give _render_leaderboard_sections an early branch for "detail" in data that renders the opener's response groups instead (this is likely what a user typing view --leaderboard CRANE would expect), or add a CLI-level guard in erd_search.py rejecting a branch target for --leaderboard.

Comment thread tests/test_report_client.py Outdated
leaderboard.data.counts.complete = 12;
}
const report = structuredClone(leaderboard);
if (allowChangedReport) report.data.rows[0].erd = 9.876;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test coverage gap: this "changed poll" simulation no longer changes anything, so the test passes vacuously.

A genuine leaderboard payload no longer has a rows key (it's columns now — confirmed by report_model.py's data.update({..., "columns": columns}) and by the fixture files). report.data.rows is therefore undefined, so report.data.rows[0].erd = 9.876 throws a TypeError inside the mocked-fetch .then() handler. That rejects the fetch promise, fetchReport's catch runs with failureCount still below the visible-failure threshold, and no re-render happens at all. The test's later assertions (that the card's position/selection are unchanged) then pass because nothing changed on screen, not because the "changed poll" code path was exercised.

The sibling test test_leaderboard_poll_renders_changed_data (just above, line 1946) was correctly migrated to report.data.columns.erd_numerator[0] = 987; — this one and its twin test_leaderboard_selection_survives_a_changed_poll (elsewhere in this file, report.data.rows[0].erd = 9.876;) appear to have been missed during the migration.

Per AGENTS.md's "Before committing and pushing": "Prove a new test can fail... A test that passes both ways proves nothing... Build fixtures in the shape production actually leaves behind."

Suggested fix: change both occurrences to mutate report.data.columns.erd_numerator[0] instead of report.data.rows[0].erd.

Comment on lines +5 to +28
"candidate_count": 14855,
"columns": {
"erd_decimal": [
null,
3.7124567
],
"erd_denominator": 100,
"erd_numerator": [
356,
null
],
"max_remaining_depth": [
6,
5
],
"word_is_answer_bitmap": "Ag==",
"word_width": 5,
"words": "saletcrane"
},
"counts": {
"complete": 2,
"infeasible": 1,
"pending": 14852
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CLAUDE.md violation: this fixture encodes a response shape production code can never emit.

The real single-opener detail path in report_model.py (collect_leaderboard_report's detail_word branch) returns only response_pattern_count and detail — it returns immediately before the columns/counts/candidate_count/total_rows block is ever built. This PR's own test pins that: test_leaderboard_carries_one_opener_s_groups_when_it_is_named and test_a_named_opener_costs_no_ranking in tests/test_report_model.py both assert assertNotIn("columns", data) for this exact request shape. Yet this fixture carries candidate_count, columns, and counts alongside detail.

Separately, columns.erd_decimal (lines 7-10) is a field _leaderboard_columns never emits — it raises ValueError on an off-lattice ERD rather than ever falling back to a decimal, which is exactly what test_a_ranking_carries_no_decimal_beside_its_numerators asserts. Pairing "erd_numerator": [356, null] with "erd_decimal": [null, 3.7124567] describes a fallback scheme the shipped code doesn't have.

Per AGENTS.md: "Build fixtures in the shape production actually leaves behind. Stopping a setup helper one call short of what the code does yields a state the system never reaches, and every assertion built on it is then testing fiction."

This is currently inert (the client's detail-open handler only reads .data.detail and discards the rest), but it misrepresents what the real server can produce and will mislead the next person who extends this fixture. Suggested fix: trim this fixture down to the shape collect_leaderboard_report actually returns for a word-detail request — just response_pattern_count and detail.

@ahernsean

Copy link
Copy Markdown
Owner Author

@codex, another review, pls

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16d4d5e45a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread report_model.py
cache = ScoreCache(
sources.cache_path, all_answers, checkpoint_on_close=False
)
if detail_word:

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 Badge Preserve text rendering for named leaderboard requests

When erd_search.py view --leaderboard crane uses the default text format, this early return supplies only data.detail, but _render_leaderboard_sections immediately reads data["counts"], candidate_count, and total_rows, so the command raises KeyError instead of rendering. The CLI accepts the positional word and the repository explicitly supports semantic word selection, so either render the detail shape in the terminal or keep this detail-only response scoped to the HTTP card endpoint.

AGENTS.md reference: AGENTS.md:L15-L18

Useful? React with 👍 / 👎.

ahernsean and others added 5 commits September 23, 2026 14:19
… screen

Four review findings.

`view --leaderboard WORD` crashed with KeyError: 'counts'.  A named opener
returns its breakdown and no ranking, so the renderer's unconditional reads of
counts, candidate_count and total_rows found none of them.  The CLI accepts the
combination, so it renders it: the opener's response groups with each group's
share of the answer list, and a plain line when its tree is unfinished.

A 304 could leave another view's report on screen.  The client keeps one
rendered report and deliberately leaves the previous view up while the next is
fetched, so revalidating the view being *entered* answered 304 for it and left
the other view's report under its tab for up to REPORT_CACHE_MAX_AGE_SECONDS.
If-None-Match is now sent only when the context matches the one on screen,
which is exactly the steady-state poll and costs it nothing.

Two changed-poll tests had stopped changing anything.  They mutated
`data.rows[0].erd`, and a ranking has no rows, so the mutation threw inside the
fetch handler, the promise rejected, no render happened, and the assertions
passed because nothing had moved.  Both now mutate the numerator, and both
assert the change reached the page -- which caught that the selection test
needs a second, different value, because a poll the client suppresses still
banks its report as the comparison baseline.

The detail fixture described a response the server cannot produce: it carried
columns, counts and candidate_count beside detail, and an erd_decimal the
ranking no longer emits at all.  Trimmed to what the detail branch returns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
Every pull-request push started a review, and a review is a Claude session
against the whole diff -- the cost lands on the repository owner's token budget
whether or not that push warranted one.  On a branch that sees a dozen pushes
while its findings are addressed, most of those reviews re-read a diff that has
barely moved.

The workflow is otherwise unchanged and still does the same review; only its
trigger moves to workflow_dispatch, taking a pull-request number as input since
a manual run carries no pull_request payload to read one from.  The original
trigger is kept commented above it, so restoring the automatic behaviour is one
edit.

  gh workflow run claude-code-review.yml --ref <branch> -f pull_request=<number>

Codex's review is unaffected and continues to run automatically.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
… into claude/leaderboard-level-of-detail

* origin/claude/disable-automatic-review:
  Run the Claude code review on request rather than on every push
The comment was written before the input existed and was not revisited.  As it
stood, an operator copying it dispatched without `pull_request`, and GitHub
refuses the request outright -- so the documented way to start a review could
not start one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
… into claude/leaderboard-level-of-detail

* origin/claude/disable-automatic-review:
  Name the required input in the documented dispatch command
@ahernsean

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: afc56161a6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread report_client.html
Comment thread report_model.py
"total_rows": len(ranked),
"matched_rows": len(ranked),
"answer_count": answer_count,
"columns": columns,

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 Badge Bump the schema version for the columnar payload

This replaces the version-4 leaderboard's rows array with an incompatible columns representation while leaving SCHEMA_VERSION at 4. A browser page or JSON consumer kept open across a server upgrade therefore accepts the response as compatible; the previous browser code evaluates data.rows || [] and silently displays an empty leaderboard. Increment the schema version so incompatible clients reject the response rather than misrendering it.

Useful? React with 👍 / 👎.

A poll never replaces the report while a control inside it holds focus,
because replaceChildren would delete the node under the reader's finger.
Opening a card focuses its summary, so that branch is now on the
leaderboard's ordinary path rather than the tree view's rare one.

The suppressed report is still banked, and correctly: lastReport and the
stored entity tag describe what the server holds.  Nothing then noticed
that the screen was a report behind -- the next poll revalidated against
the new tag and got 304, or compared the same body against the banked
one and found it unchanged -- so the ranking never arrived, and with the
queue stopped it never would.

The debt is recorded and settled by the first poll that finds the
interaction over, redrawing from the report the client already holds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LdvsAVg5huEvfszehHCqjN
@ahernsean
ahernsean merged commit d84aada into main Oct 3, 2026
7 checks passed
@ahernsean
ahernsean deleted the claude/leaderboard-level-of-detail branch October 3, 2026 04:45
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.

Leaderboard ships every row's full response-group breakdown, projecting to 62.9 MB

1 participant