Repository navigation
docs: rebuild the showcase layout and refresh the comparison board - #555
Conversation
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: update the PR description to reflect the replacement comparison board and its verified dimensions.
The README changes themselves render cleanly, and I found no source-diff defect. The PR description is now stale after the second commit: Summary calls the changes "all structural"; Verification still lists the Frameworks artifact as 422x281; and Risk says no image was added or replaced and that all attachment URLs and alt text are unchanged. Commit 573669983d77 replaces one attachment and its alt text, and the new 2000x1547 board at 86% renders about 363x281 beside the 422x281 sweep image. Under AGENTS.md section 3.7, the PR body becomes the squash-commit body and must summarize the overall multi-commit goal and report exact verification and risk. Please update those sections before merge.
Review coverage: the full diff and surrounding README; file history and both commits; AGENTS.md/CLAUDE.md documentation, asset, source-language, commit, and PR-description rules; backward compatibility of links and alt text; GitHub's GFM rendering; and test impact. No runtime callers or architecture boundaries are involved, and no tests were changed or weakened.
Verification: git diff --check passed; both repository gate scripts passed when run directly through uv (make is unavailable in this review environment); GitHub rendered one table with 12 rows, 24 cells, and 16 images; and I downloaded and inspected the new 2000x1547 PNG and independently checked its height alignment against the 2000x1332 sweep image. No pytest suite is relevant to this README-only change; the latest GitHub run had docs build, repository files, page checks, commit-message checks, PR-title checks, and kernel wheel smoke passing, with the broader suite still pending at review time.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior PR-description blocker is resolved: the description now accurately covers the replacement board, revised alt text, 363x281 rendered measurement, and rollback scope. The new delta cleanly splits the showcase into one three-row table per pair, and GitHub's GFM renderer preserves four tables with three rows, six cells, and four images each. I found no new defect.
Review coverage: the full current diff and the delta from 573669983d77; surrounding README content and commit history; AGENTS.md/CLAUDE.md documentation, asset, source-language, commit, and PR-description rules; attachment and alt-text compatibility; rendered HTML structure; and test impact. There are no runtime callers or architecture boundaries involved, and no tests were changed or weakened. No review threads exist to resolve.
Verification: git diff --check passed; check_large_files.py and check_source_language.py passed via the repository's uv command; the PR description passed the required ASCII scan; source invariants confirm 4 tables, 12 rows, 24 cells, 16 images, and exactly one intentional attachment replacement; and GitHub's markdown endpoint rendered the expected four-table structure. No pytest suite is relevant to this README-only change. At review time, all reported CI jobs passed except three broader unit-test shards that were still pending.
|
Not a blocker -- an A stands beside this, and it is about what the merge produces rather than #557 merged after this branch was cut and gave README.zh-CN.md the Showcase section, carrying the So README.md gets the redrawn three-way-tagged board and README.zh-CN.md keeps the two-way one. Nothing here is this PR's doing. The branch predates #557, the merge is clean Everything the description claims, I checked and it holds:
|
|
Blocking: this revision must not merge as it stands. Rebase it onto current Scope: this PR changes one file, What the PR does: rebuilds the README showcase so each pair of cases gets its own table -- a Why the rebase has to come first: this branch forked at 8d74fb9, and its own tree does not The blocker is at the merged product. I merged this head onto current main (e708f34) in a
0xKT measured the same numbers on this PR at 11:37 and read them as not a blocker. That Verification:
Checked and deliberately not reported: the width arithmetic (86% of 2000x1547 against 100% of |
gloryfromca
left a comment
There was a problem hiding this comment.
Blocking: rebase onto current main and update README.zh-CN.md to the same comparison board and three-way alt text.
This revision does not resolve the two existing reviewer blockers. It incorporates main through merge commit 08d954dc548c rather than the repository-required rebase, and its diff against current main still changes only README.md. The resulting tree has the new 5120c2c0 board and three-way description in README.md, while README.zh-CN.md still has the old 75beba36 board and two-way description. I am not adding a duplicate inline finding or resolving 0xKT's open thread; that thread remains accurate and belongs to its author.
Everything else remains sound. Review coverage included the full current diff, the delta from 2b73f5c37deb, merge history, both front pages, AGENTS.md/CLAUDE.md branch and documentation rules, backward compatibility, test changes, and architecture impact. Relative to current main, no runtime code, tests, callers, or architecture boundaries change, and no tests were weakened.
Verification: git diff --check passed; check_large_files.py and check_source_language.py passed through the repository's uv command; the asset IDs were confirmed directly in both front pages; and GitHub CI was passing for all reported jobs except unit shard 4/4, which was still pending at review time. No local pytest target is relevant to the README-only PR diff.
08d954d to
20dcf2c
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The blocker I held on the previous revision is resolved. This head is linearly rebased on current main, has no merge commits in the PR range, and updates README.zh-CN.md alongside README.md. Both front pages now use the new comparison board, identical three-way alt text, and the same aligned table structure while preserving the translated headings and the game table from #557. I found no new defect.
Review coverage: the complete two-file diff and the delta from 08d954dc548c; branch and commit history; both rendered front pages; AGENTS.md/CLAUDE.md branch, documentation, asset, source-language, commit, and PR-description rules; attachment and alt-text compatibility; test changes; and architecture impact. No runtime code, callers, tests, or architecture boundaries change, and no tests were weakened. I own no review thread on this PR; 0xKT's now-answered thread remains for 0xKT to resolve.
Verification: git diff --check, check_large_files.py, check_source_language.py, and the PR-body ASCII scan passed. Source checks confirmed both pages have 5 tables, 13 rows, 17 images, the same 18 attachment IDs, the new board, and matching alt text. GitHub's markdown endpoint rendered each page as the game table plus four three-row pair tables. No local pytest target is relevant to this README-only change. All reported CI jobs passed except the four broader unit shards, which were still pending at review time.
The section had grown to nine cases across three tables of mixed shape, and the layout no longer held them. A heading that wrapped to two lines pushed its column's images down, paired task graphs of unequal height left the artifacts below them starting at different heights, and GitHub's row striping drifted against a case's height, so headings landed on white or on the grey depending on where they fell. Each case now spans three rows - heading, task graph, artifact - and each pair gets its own table, which restarts the stripe and puts a gap between pairs. The game run keeps the opening slot and now takes the full width rather than leaving half a row empty. Image widths are set per pair so both sides render to one height, and the narrower image is centred. The divider sentence between the old tables is gone; the headings already say it. The Frameworks artifact is replaced with a redrawn plate: every framework carries its vendor mark and the orchestration style became a three-way tag where the old one split the six in two, so its alt text names three categories instead of two. Both READMEs get the same treatment. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
20dcf2c to
ed9d714
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new delta is sound. The game showcase now uses the same three-row pattern as every paired table and takes the full width in both READMEs; GitHub's renderer still emits the attachment as a video player. The branch remains linearly based on current main, both front pages retain the same attachment set and comparison-board description, and I found no new defect.
Review coverage: the complete current diff and the delta from 20dcf2ccdc6e; branch and commit history; both rendered front pages; AGENTS.md/CLAUDE.md branch, documentation, asset, source-language, commit, and PR-description rules; attachment compatibility; test changes; and architecture impact. No runtime code, callers, tests, or architecture boundaries change, and no tests were weakened. I own no review thread; 0xKT's answered thread remains for 0xKT to resolve.
Verification: git diff --check, check_large_files.py, check_source_language.py, and the PR-body ASCII scan passed. Source checks confirmed both pages have five three-row tables, 15 rows, 17 images, identical 18-ID attachment sets, and exactly one asset swap. GitHub's markdown endpoint rendered both expected table structures and a video player for the game attachment. No local pytest target is relevant to this documentation-only change. All reported CI jobs passed except unit shard 4/4, which was still pending at review time.
## Summary The parameter sweep case shipped a two-panel matplotlib plot. The same run also produced a dashboard, and it carries what the plot could only assert: the sixteen measured cells as a grid with recall and latency in each, the best cell ringed at top_k 10 and chunk_size 1024, and a footnote naming the single warm-up sample behind the 256/top_k=3 outlier. This swaps the plot for the dashboard and rewrites the alt text to describe that grid rather than the two lines. **Language.** The dashboard was authored in Chinese. Both front pages share one image set, and the comparison board beside this cell is already English, as is the alt text on both pages, so what lands is an English render built the same way that board was. The Chinese original is left untouched on disk and nothing in the repository refers to it. **Widths.** The new render is 2000x1568 against the board's 2000x1547, so the two sit almost exactly on one aspect. The board goes back to full width and the dashboard takes 99 percent; the board only needed 86 percent against the old plot's 2000x1332. ## Type - [ ] Fix - [ ] Feature - [x] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification ``` git diff --check origin/main...HEAD clean COMMIT_RANGE=origin/main...HEAD make check-large-files exit 0 PYTHONPATH=. uv run --frozen --python 3.12 --extra dev python scripts/check_source_language.py origin/main...HEAD exit 0 make check-commits exit 0 PR_TITLE="docs: replace the sweep chart with the run's dashboard" make check-pr-title exit 0 git merge-tree --write-tree HEAD origin/main clean ``` The attachment was fetched anonymously before it was referenced: HTTP 200, 2000x1568, md5 fcd434b3000cbcda9339e98c22bf974b, matching the local render byte for byte. Rendered the section through GitHub's markdown endpoint under the published markdown stylesheet and measured every image box. All eight pairs share a top edge and match within 2px on height; the Frameworks pair is now 379x293 against 375x294, where before this change it was 326x252 against 375x294. Both front pages were compared after the edit: each carries the identical set of attachment ids, and neither still references the old plot. No test suite is relevant to a change that edits two markdown files. The python lint job was not run locally for the same reason; CI runs it on the head. - [ ] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk Documentation only, in both READMEs. One attachment reference is replaced and one width attribute is restored; no file is committed to the repository. The old plot remains reachable at its own attachment URL, so a revert of the commit restores it with no other action. - [ ] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues Follows #555. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
Rebuilds the Showcase layout in both READMEs, and replaces one image.
Why the old layout did not hold. The section had grown to nine cases
across three tables of mixed shape. A heading that wrapped to two lines
pushed its column's images down. Paired task graphs of unequal height left
the artifacts below them starting at different heights. And GitHub's
stylesheet stripes every second row while a case spanned three, so the
pattern drifted until some headings sat on white and others on the grey.
What it is now. Each case spans three rows - heading, task graph,
artifact - so cells in a row share a top edge and a long heading cannot
cascade into the images. Each pair gets its own table, which restarts the
stripe and puts the usual gap between pairs. The game run keeps the opening
slot and now takes the full width instead of leaving half a row empty.
Image widths are set per pair so both sides render to one height, and the
narrower image is centred. The divider sentence that separated the old
tables is gone, since the headings already say it.
A new comparison board. The Frameworks artifact is replaced with a
redrawn plate: each framework carries its vendor mark, and the
orchestration style became a three-way tag - explicit graph or canvas,
declared task flow, dynamic at runtime - where the old plate split the six
in two. Its alt text said "versus" and named two categories; it names three
now. The render is 2000x1547 against the old 2000x1332, so its width is set
to 86 percent to stay level with the sweep chart beside it.
Type
Verification
make check-large-files-> exit 0make check-source-language-> exit 0The replacement board was fetched anonymously from its attachment URL:
HTTP 200, 2000x1547, and its md5 matched the local source file byte for
byte.
Rendered the section through GitHub's own markdown endpoint under the
published markdown stylesheet and read back the computed row backgrounds.
All five heading rows now report one background; before the split they
reported two.
Measured every image box in that render. All four pairs match on top edge,
and within 2px on height:
Confirmed against the repository-rendered README that the game run's bare
attachment URL is served as a video player, and kept it on its own
paragraph inside the cell so it still is.
Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
Documentation only, in both READMEs. One of the seventeen attachments is
replaced, with its alt text rewritten to match the new plate's three-way
legend; the rest are untouched, and no file is committed to the repository.
Rollback is reverting the commit.
Related Issues
Follows #547 and #557.