Repository navigation
Let the holistic judge show Commons images - #616
Conversation
A list may now hold a strip of Commons pictures, shown as one entry `[IMAGES from Wikimedia Commons] 1. <file> — <description> | 2. ...`, or a single picture shown as `[IMAGE] <file> — <description>`. A strip adds to the ten results unless it is marked as displacing one. Only batches that show images get a paragraph telling the judge about them and to judge whether they help the query, plus a line to open the thumbnails when there are any; the rubric is otherwise unchanged. Every batch now names the absolute path its judge writes to. Part of #610 No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| "extract"} object. Only the top ten count. X is the arm under test and Y the baseline, so | ||
| a positive preference favours X. | ||
|
|
||
| A list may also hold Wikimedia Commons pictures. A single picture, {"url", "title", "extract", |
There was a problem hiding this comment.
We should be careful about properly escaping title/description. These are random-user-entered values and potentially can cause XSS attacks etc.
There was a problem hiding this comment.
Reply from Claude: Changed in 7503d71 and d6ef9d9. There's no XSS here: this text only goes into a prompt file that an LLM reads offline, and nothing renders it as HTML. The two fields should still be normalised the same way, and now they are. Commons titles and descriptions both go through a new commons_text. It decodes HTML entities and also strips any tags that only appear once those entities are decoded (e.g. <script>). Web extracts are handled as before. On the measured data, no Commons title or description contained a tag or entity, so this changes nothing there.
Stripping markup doesn't stop a description that addresses the judge. In this offline eval such a description could sway at most one verdict, since each comparison is judged in both orders. For production, the front end must render Commons text escaped and never as HTML, and that's in the doc's production notes.
|
What happens if we show a Wikipedia entry, too? Will we see the Wikipedia entry + image (if it has one), and then potentially a strip or single image? |
|
|
||
| NUM_RESULTS = 10 | ||
| EXTRACT_CHARS = 400 | ||
| IMAGE_DESCRIPTION_CHARS = 150 |
There was a problem hiding this comment.
Are we chunking properly and not stopping mid-word or mid-sentence?
There was a problem hiding this comment.
Reply from Claude: Fixed in 7503d71. shortened() cuts at the last sentence end within the 150-character limit if one falls in its second half. Otherwise it cuts at the last word boundary and adds "…". The measured runs in #615 used the old mid-word cut: 19 of A2's 59 strips and 37 of A1's 191 images would now read differently. I haven't judged them again, because the change only trims a partial last word or sentence, and the doc says the runs used the old text.
|
Review: PR #616 — Let the holistic judge show Commons images I found one definite issue in the diff, plus two areas worth tightening before merging.
The reviewer's security concern is valid. image_text() inserts Wikimedia Commons titles and descriptions directly into the judge's input: python These values originate outside the trusted application. Even though descriptions have HTML tags stripped, this isn't a complete sanitisation strategy. In particular, HTML entities are decoded afterwards, and titles aren't processed by plain_text() at all. I'd recommend consistently normalising both fields and ensuring that untrusted metadata cannot introduce unintended markup or instructions into the judge's input. One important distinction: this is primarily an untrusted-input/prompt-integrity concern in this context; HTML escaping alone is not a complete defence against prompt injection.
Currently: python The 150-character limit is sensible, but slicing can stop halfway through a word or sentence. For example, a description might contain the subject in its final few words, which would be lost. I'd truncate at the last word boundary within the limit. Ideally, prefer a sentence boundary where practical. This also addresses the reviewer's question about chunking.
In differing_queries(): python Comparisons are selected based on URL differences alone. This is potentially problematic for image strips: if two arms have the same synthetic strip URL but different images or descriptions, the query won't be judged. I'd compare the actual rendered content of image entries as well as URLs. This is particularly important for an evaluation intended to measure the quality of image additions. |
…ontent Commons titles now go through the same plain-text normalisation as descriptions, and entities are decoded before tags are stripped, so an encoded tag is stripped too. Descriptions are shortened at a sentence end or word boundary instead of mid-word. Image entries are compared by what the judge is shown, so two strips with one URL but different pictures still count as differing. The measured runs are unaffected in which queries they compare; 19 of A2's 59 strips would read differently because of the new shortening. No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… file Decoding entities before stripping tags also cut text out of web extracts, so "a < b and c > d" lost everything between the brackets. Web extracts go back to stripping tags first; only Commons text strips the tags that decoding reveals, with a pattern that matches tag shapes alone and can't run quadratically on unclosed brackets. Image entries now compare by URL as well as text, so two files with the same text still differ. No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No functional changes to the crawler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Reply from Claude: Replying to @blueheron786's review above (6751849160). [P1] Untrusted metadata and [P2] truncation: see the replies on the inline threads. Titles and descriptions now go through [P3] Image-only changes. Fixed in 7503d71 and d6ef9d9. Web results are still compared by URL, so earlier Combined Search evals select the same queries. Image entries are now compared by URL plus the text the judge is shown. In the measured runs this doesn't change which queries were compared. Rerunning the selection offline for A1, A2, A2-displace and A2-vision against A0 gives exactly the same queries (118/59/59/20). Every measured comparison was against A0, which has no image entries, and strip URLs are built from the images' page URLs. The one place it would matter is A2-vision against A2, which was never run. |
|
Reply from Claude: Replying to the Wikipedia question above (6751684614). Combined Search doesn't add Wikipedia separately. Wikipedia pages come back as ordinary Staan results, and nothing deduplicates them against the strip. The front end ( There's now a production note in the doc. Drop from the strip any image that is the card's lead image, matched by the last segment of the summary's |
Stack
PR 4 of 7 in the #610 evaluation stack: #611 ← #612 ← #613 ← #616 ← #615 ← #617 ← #614. Merge in that order.
I split this out of #615 so each PR stays under 500 added lines.
What changed
mwmbl/rankeval/evaluation/holistic_judge.pycan now show Wikimedia Commons pictures in a list:[IMAGES from Wikimedia Commons] 1. <file name> — <description> | 2. …. Descriptions are cut to 150 characters. A thumbnail path is added when one is given.{"url", "title", "extract", "image": true}, rendered as[IMAGE] <file name> — <description>."displaces": true.Results: what the judge sees
[IMAGES from Wikimedia Commons] 1. <file name> — <description> | 2. …"displaces": true[IMAGE] <file name> — <description>The rubric is otherwise unchanged. Batches without images get the same prompt as before, apart from the output-path line. #615 uses this to judge the arms.
How verified
make checkpasses.test/test_holistic_judge.pypasses 12 tests, three of them new. They cover strip and single-image rendering, a strip not counting towards ten, a displacing strip counting towards ten, and the image notes appearing only in batches that show images.mwmbl/rankeval/seed_images/*is omitted from coverage (set in Add a harness to evaluate images in Seed Search #611), because it is offline evaluation code.Decisions already made
Nothing outside
mwmbl/rankevalimports this code, so production is unchanged.Part of #610
🤖 Generated with Claude Code