Publish recorded benchmark results, and name the OCR model and tokens for every run - #31
Publish recorded benchmark results, and name the OCR model and tokens for every run#31damodaha wants to merge 2 commits into
Conversation
The OCR section listed the authentication options abstractly but not the two setups people actually reach for. Add both, verified against the code that reads them (`_resolve_ocr_env`): a vision model served on your own machine, and Bedrock behind a translating proxy. Bedrock gets explicit treatment because likhit speaks only the OpenAI chat API -- there is no Bedrock support in the package -- so pointing it straight at Bedrock cannot work, and a reader following the generic "custom base URL" option would find that out the hard way. Also record when OCR runs at all. It fires only for pages a text layer cannot serve, so a configured backend does not mean paying for one per document: 13 of the benchmark's 16 documents convert without a single vision call. And note that leaving OCR unconfigured degrades rather than fails, quoting the log line likhit actually emits.
… tokens CI cannot measure this benchmark. A runner has no vision backend, so a live build could only ever exercise the no-OCR configuration -- 16 runs with 32 skipped -- and converting the full corpus three ways takes far longer than a Pages build. Record a complete local run instead and replay it. `--write-snapshot` records every run's status, text, timing, memory and OCR usage; `--snapshot` replays them, converting nothing and calling no backend. Availability comes from the recording rather than the environment, or CI would drop the very OCR columns the snapshot exists to publish. Everything downstream is recomputed from the recorded text, so a catalog edit takes effect without re-recording -- which is how the `npc-press-note` expectation below applies to an existing snapshot. The property this rests on is that a replay is indistinguishable from the run it recorded, verified against the real build: identical summary, metrics, checks and artifact hashes, and byte-identical transcripts and diagnostics across all 48 runs. That is why the recording keeps the raw stderr rather than the composed diagnostic file -- composing it twice would append the error lines again. Provenance is deliberately split: `build` is the commit that published the page, `measured` the commit whose behaviour the numbers describe. `measured.stale` flags a divergence and `missing_runs` names catalog runs the recording predates; the dashboard renders both above the summary, and run metadata attributes the conversion to the recorded environment rather than the publishing one. A live build sets `measured` to null. OCR reporting was misleading in two ways, both fixed here: - `_ocr_usage_record` returned null for a run that made no vision call, which is most of them -- likhit only calls a model for pages a text layer cannot serve. That made "spent nothing" indistinguishable from "nobody was counting", and the dashboard drew a blank for both. Zero is now a measurement; null means only that the counter was unreachable. - The model id lived inside the per-run usage record, so it was absent for exactly those runs, and for any backend without a counter. It is now recorded per configuration and shown beside the configuration selector. `ocr_usage_proxy.py` is a reference implementation of the counter contract the README documented but nothing shipped: it forwards to any OpenAI-compatible endpoint and accumulates the usage block each response carries. One per backend, so a hosted run cannot be credited with a locally served model's spend. `summarize.py` writes the published numbers, per-configuration outcomes, models and token totals to the Actions job summary, so a regression is visible on the run itself rather than only in the deployed page. Two behaviour notes: - `npc-press-note` under offline OCR exhausted the 2400s budget on one recording and completed in 1073s on the next, so it is intermittently slow rather than broken. Marked `known_issue`: a timeout is reported instead of failing the build, and a run that completes is promoted back to pass on its own, which is what the committed recording shows. - A document whose every run is skipped is now dropped rather than published with an empty `runs` array, which the schema forbids. Latent before this change.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe benchmark site now supports versioned snapshot recording and replay, OCR usage tracking, model metadata, measured-result provenance, dashboard disclosure, and GitHub Actions summaries. Documentation and tests cover OCR setup, replay validation, unavailable backends, and reporting behavior. ChangesBenchmark site
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
is_auto_command: True
custom_model_max_tokens: 200000
git_provider: github
output_relevant_configurations: True
model: openai/cx/gpt-5.5
ENABLE_AUTO_APPROVAL: True
custom_reasoning_model: False
fallback_models: ['openai/cx/gpt-5.4-mini']
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_reviewer] require_ticket_analysis_review: False
require_score_review: False
require_tests_review: True
require_estimate_effort_to_review: True
require_can_be_split_review: False
require_security_review: True
require_estimate_contribution_time_cost: False
require_todo_scan: False
publish_output_no_suggestions: True
persistent_comment: True
extra_instructions:
num_max_findings: 3
final_update_message: True
enable_review_labels_security: True
enable_review_labels_effort: True
require_all_thresholds_for_incremental_review: False
minimal_commits_for_incremental_review: 0
minimal_minutes_for_incremental_review: 0
enable_intro_text: True
enable_help_text: False
|
PR Code Suggestions ✨Explore these optional code suggestions:
🛠️ Relevant configurations:
[config] enable_ai_metadata: False
is_auto_command: True
custom_model_max_tokens: 200000
git_provider: github
output_relevant_configurations: True
model: openai/cx/gpt-5.5
ENABLE_AUTO_APPROVAL: True
custom_reasoning_model: False
fallback_models: ['openai/cx/gpt-5.4-mini']
publish_output: True
publish_output_progress: True
progress_gif_url:
progress_gif_width: 48
verbosity_level: 0
use_extra_bad_extensions: False
log_level: DEBUG
use_wiki_settings_file: True
use_repo_settings_file: True
use_global_settings_file: True
extra_config_url:
disable_auto_feedback: False
ai_timeout: 120
response_language: en-US
repo_context_files: ['AGENTS.md']
repo_context_from_default_branch: True
repo_context_max_lines: 500
max_description_tokens: 500
max_commits_tokens: 500
max_model_tokens: 32000
model_token_count_estimate_factor: 0.3
patch_extension_skip_types: ['.md', '.txt']
allow_dynamic_context: True
max_extra_lines_before_dynamic_context: 10
patch_extra_lines_before: 5
patch_extra_lines_after: 1
cli_mode: False
large_patch_policy: clip
duplicate_prompt_examples: False
seed: -1
temperature: 0.2
ignore_pr_title: ['^\\[Auto\\]', '^Auto']
ignore_pr_target_branches: []
ignore_pr_source_branches: []
ignore_pr_labels: []
ignore_pr_authors: []
ignore_repositories: []
ignore_language_framework: []
restricted_mode: False
reasoning_effort: medium
enable_claude_extended_thinking: False
extended_thinking_budget_tokens: 2048
extended_thinking_max_output_tokens: 4096
claude_extended_thinking_models_override: []
extract_issue_from_branch: True
branch_issue_regex:
enable_custom_labels: False
[pr_code_suggestions] commitable_code_suggestions: False
dual_publishing_score_threshold: -1
focus_only_on_problems: True
extra_instructions:
enable_help_text: False
enable_chat_text: False
persistent_comment: True
max_history_len: 4
publish_output_no_suggestions: True
suggestions_score_threshold: 0
new_score_mechanism: True
new_score_mechanism_th_high: 9
new_score_mechanism_th_medium: 7
auto_extended_mode: True
num_code_suggestions_per_chunk: 3
max_number_of_calls: 3
parallel_calls: True
final_clip_factor: 0.8
decouple_hunks: False
demand_code_suggestions_self_review: False
code_suggestions_self_review_text: **Author self-review**: I have reviewed the PR code suggestions, and addressed the relevant ones.
approve_pr_on_self_review: False
fold_suggestions_on_self_review: True
|
PR Agent Walkthrough 🤖Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more. Here is a list of tools you can use to interact with the PR Agent:
(1) Note that each tool can be triggered automatically when a new PR is opened, or called manually by commenting on a PR. (2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the |
|
Auto-approved PR |
![jawafdehi-pr-agent[bot]](https://avatars.githubusercontent.com/u/241969530?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
README.md (1)
186-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fenced block.
markdownlint reports MD040 for this fence. The block contains log output, so
textis the correct tag.♻️ Proposed fix
-``` +```text PDF converter: OCR appears necessary, but OCR is not configured. Set OPENAI_API_KEY or GEMINI_API_KEY, plus MARKITDOWN_OCR_MODEL, to enable markitdown-ocr.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 186 - 189, Update the fenced block containing the PDF
converter OCR message to specify the text language tag, preserving its existing
log output content.</details> <!-- cr-comment:v1:3fac2a4eb1138458d2ff7e54 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>tests/test_benchmark_site.py (1)</summary><blockquote> `1546-1547`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _💤 Low value_ **Pass an unreachable `run_case` to this validation test.** `generate` is called without `run_case`, so it falls back to `_default_run_case`. Validation currently raises before any conversion, so the test passes. If that ordering regressed, this test would run real subprocess conversions of the synthetic corpus before failing, which turns a clear assertion failure into a slow and confusing one. <details> <summary>♻️ Proposed change</summary> ```diff with pytest.raises(ValueError, match=expected): - generator.generate(tmp_path / "site", include_public=False, snapshot=snapshot) + generator.generate( + tmp_path / "site", + include_public=False, + snapshot=snapshot, + run_case=_unreachable_run_case, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_benchmark_site.py` around lines 1546 - 1547, Update the validation test around generator.generate to pass an unreachable run_case callback explicitly, ensuring validation fails before any subprocess conversion is attempted. Keep the existing ValueError assertion and synthetic corpus setup unchanged.site/generate.py (2)
644-663: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider validating the per-run record type too.
_load_snapshotchecks thatrunsis a dict, but not that each value is a dict. A hand-edited snapshot with a string value reaches_replayed_payload, whererecorded[field]raisesTypeErrorandrecorded.getraisesAttributeErrordeep in the build. That is the same failure class the comment on Line 659 already guards against one level up.♻️ Proposed check
for field in ("build", "configurations", "runs"): # Typed, not merely present: a `runs` string would pass a presence check # and then fail deep in the build with an AttributeError. if not isinstance(snapshot.get(field), dict): raise ValueError(f"{path} is missing required field {field!r}") + for key, recorded in snapshot["runs"].items(): + if not isinstance(recorded, dict): + raise ValueError(f"{path} records run {key!r} as {type(recorded).__name__}") return snapshot🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/generate.py` around lines 644 - 663, Update _load_snapshot to validate that every value in the snapshot’s runs mapping is a dict before returning the snapshot. Raise the same ValueError style used for malformed top-level fields, so invalid per-run records are rejected before _replayed_payload processes them.
975-979: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winEnforce the snapshot/write-snapshot exclusion in
generate, not only in the parser.The comment on Line 1187 states that reading a recording while writing one produces a copy with a fresh timestamp and no new measurement.
build_parserenforces that for the CLI.generatedoes not: a caller can pass bothsnapshotandwrite_snapshot, and the resulting file records the replaying build's commit and a newrecorded_atover recorded numbers. Move the invariant into the function so it holds for every caller.♻️ Proposed guard
catalog = _load_catalog() + if snapshot is not None and write_snapshot is not None: + raise ValueError( + "snapshot and write_snapshot are mutually exclusive: recording a " + "replay would produce a copy with a fresh timestamp and no new " + "measurement" + ) snapshot_data = _load_snapshot(snapshot) if snapshot is not None else None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/generate.py` around lines 975 - 979, Update generate to reject calls where both snapshot and write_snapshot are provided, before loading the catalog or snapshot. Preserve the existing CLI parser validation, but enforce the same mutual-exclusion invariant directly in generate for all callers.site/ocr_usage_proxy.py (1)
171-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the
--upstreamscheme at startup.
--upstreamis accepted as any string. If an operator omits the scheme,urllib.request.urlopenraisesValueError: unknown url type. That exception is not in the caught tuple in_proxy, so every request fails with a 500 and no clear cause. Reject the value at startup instead.♻️ Proposed validation
args = parser.parse_args(argv) + if not args.upstream.startswith(("http://", "https://")): + parser.error("--upstream must start with http:// or https://") + counter = Counter()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/ocr_usage_proxy.py` around lines 171 - 189, Validate args.upstream in main before constructing ThreadingHTTPServer, requiring an explicit supported URL scheme such as http or https and rejecting missing or unsupported schemes through the argument parser with a clear error. Keep build_handler and request forwarding unchanged for valid upstream values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@site/generate.py`:
- Around line 1008-1017: The replay configuration handling must explicitly
record catalog configurations absent from snapshot_data["configurations"]
instead of treating them as unavailable and silently dropping their runs. Update
the availability/models construction and related missing-run tracking around
recorded_configurations, preserving the existing visibility behavior used by the
Line 1051 path so unknown configurations are reported in measured.missing_runs
and remain available to dashboard reporting.
In `@site/ocr_usage_proxy.py`:
- Around line 121-126: Update the 413 response path in do_POST to close the
HTTP/1.1 connection by including the appropriate Connection: close response
header, preventing unread request-body bytes from being parsed as the next
request. Leave the existing handling of requests without Content-Length
unchanged.
In `@site/README.md`:
- Around line 100-107: Update the earlier Configurations paragraph to accurately
describe the deployed site's replay behavior, including that availability is
read from recorded results and all published columns can be shown;
alternatively, add a clear forward reference to the later replay section. Keep
the CI configuration details consistent with the explanation in the replay
section.
In `@site/summarize.py`:
- Around line 89-109: Update _configuration_rows() to track whether any
ocr_usage record was present for each configuration, separately from its call
count. Render the measured usage note whenever usage was recorded, including
zero calls and tokens; when no usage record exists, render an explicit
unavailable-usage state instead of omitting OCR information.
---
Nitpick comments:
In `@README.md`:
- Around line 186-189: Update the fenced block containing the PDF converter OCR
message to specify the text language tag, preserving its existing log output
content.
In `@site/generate.py`:
- Around line 644-663: Update _load_snapshot to validate that every value in the
snapshot’s runs mapping is a dict before returning the snapshot. Raise the same
ValueError style used for malformed top-level fields, so invalid per-run records
are rejected before _replayed_payload processes them.
- Around line 975-979: Update generate to reject calls where both snapshot and
write_snapshot are provided, before loading the catalog or snapshot. Preserve
the existing CLI parser validation, but enforce the same mutual-exclusion
invariant directly in generate for all callers.
In `@site/ocr_usage_proxy.py`:
- Around line 171-189: Validate args.upstream in main before constructing
ThreadingHTTPServer, requiring an explicit supported URL scheme such as http or
https and rejecting missing or unsupported schemes through the argument parser
with a clear error. Keep build_handler and request forwarding unchanged for
valid upstream values.
In `@tests/test_benchmark_site.py`:
- Around line 1546-1547: Update the validation test around generator.generate to
pass an unreachable run_case callback explicitly, ensuring validation fails
before any subprocess conversion is attempted. Keep the existing ValueError
assertion and synthetic corpus setup unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f639538-fabb-41a1-b89d-038897c03891
📒 Files selected for processing (13)
.github/workflows/benchmark-pages.ymlREADME.mdsite/README.mdsite/catalog.jsonsite/generate.pysite/ocr_usage_proxy.pysite/schema.jsonsite/snapshot.jsonsite/static/app.jssite/static/index.htmlsite/static/styles.csssite/summarize.pytests/test_benchmark_site.py
| else: | ||
| recorded_configurations = snapshot_data["configurations"] | ||
| availability = { | ||
| name: bool(recorded_configurations.get(name, {}).get("available")) | ||
| for name in catalog["configurations"] | ||
| } | ||
| models = { | ||
| name: recorded_configurations.get(name, {}).get("model") | ||
| for name in catalog["configurations"] | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
A configuration missing from the snapshot is skipped silently.
On replay, recorded_configurations.get(name, {}) resolves an unrecorded configuration to available=False. Its runs are then dropped at Line 1043 before the snapshot lookup at Line 1046, so they never reach missing_runs. The result is a rise in summary.skipped with nothing named in measured.missing_runs and nothing for the dashboard to report.
This is the same situation the comment on Line 1051 treats as requiring visibility. test_committed_snapshot_covers_every_published_run_and_configuration catches it for the committed file, but the generator itself still degrades quietly for any other snapshot. Consider recording the unknown configuration explicitly.
🐛 Proposed fix
else:
recorded_configurations = snapshot_data["configurations"]
+ unrecorded = sorted(
+ set(catalog["configurations"]) - set(recorded_configurations)
+ )
+ if unrecorded:
+ raise ValueError(
+ f"the snapshot does not cover configurations {unrecorded}; "
+ f"re-record on a machine where their backends work"
+ )
availability = {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else: | |
| recorded_configurations = snapshot_data["configurations"] | |
| availability = { | |
| name: bool(recorded_configurations.get(name, {}).get("available")) | |
| for name in catalog["configurations"] | |
| } | |
| models = { | |
| name: recorded_configurations.get(name, {}).get("model") | |
| for name in catalog["configurations"] | |
| } | |
| else: | |
| recorded_configurations = snapshot_data["configurations"] | |
| unrecorded = sorted( | |
| set(catalog["configurations"]) - set(recorded_configurations) | |
| ) | |
| if unrecorded: | |
| raise ValueError( | |
| f"the snapshot does not cover configurations {unrecorded}; " | |
| f"re-record on a machine where their backends work" | |
| ) | |
| availability = { | |
| name: bool(recorded_configurations.get(name, {}).get("available")) | |
| for name in catalog["configurations"] | |
| } | |
| models = { | |
| name: recorded_configurations.get(name, {}).get("model") | |
| for name in catalog["configurations"] | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/generate.py` around lines 1008 - 1017, The replay configuration handling
must explicitly record catalog configurations absent from
snapshot_data["configurations"] instead of treating them as unavailable and
silently dropping their runs. Update the availability/models construction and
related missing-run tracking around recorded_configurations, preserving the
existing visibility behavior used by the Line 1051 path so unknown
configurations are reported in measured.missing_runs and remain available to
dashboard reporting.
| def do_POST(self) -> None: # noqa: N802 - BaseHTTPRequestHandler's spelling | ||
| length = int(self.headers.get("Content-Length") or 0) | ||
| if length > MAX_BODY_BYTES: | ||
| self._respond(413, b'{"error":"request too large"}', "application/json") | ||
| return | ||
| self._proxy(self.rfile.read(length) if length else b"") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Close the connection after a 413, or drain the body.
protocol_version is HTTP/1.1, so the connection stays open by default. On the 413 path the request body is never read from self.rfile. The unread bytes remain in the socket and the next request parse reads them as a request line. Send Connection: close on that response so the client cannot reuse a corrupted connection.
Note also that a request without Content-Length (chunked transfer encoding) forwards an empty body. That is acceptable for the OpenAI-compatible clients used here, but it is silent.
🔒️ Proposed fix for the 413 path
def _respond(self, status: int, body: bytes, content_type: str) -> None:
self.send_response(status)
self.send_header("Content-Type", content_type)
self.send_header("Content-Length", str(len(body)))
+ if status == 413:
+ # The request body was never read, so the connection cannot be
+ # reused: the leftover bytes would be parsed as the next request.
+ self.send_header("Connection", "close")
+ self.close_connection = True
self.end_headers()
self.wfile.write(body)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/ocr_usage_proxy.py` around lines 121 - 126, Update the 413 response path
in do_POST to close the HTTP/1.1 connection by including the appropriate
Connection: close response header, preventing unread request-body bytes from
being parsed as the next request. Leave the existing handling of requests
without Content-Length unchanged.
| A replay performs no conversion and calls no backend. It takes each run's status, | ||
| text, timing, memory and OCR usage from the record, and takes **availability from | ||
| the record too** — otherwise CI would skip the very OCR columns the snapshot | ||
| exists to publish. Everything downstream is recomputed from the recorded text, so | ||
| editing a check threshold in `catalog.json` changes a replayed result without | ||
| re-recording. Sources are still downloaded and hash-checked, so a replayed build | ||
| still needs network access to the publisher URLs; only the conversions are | ||
| replayed. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
This section contradicts the Configurations section above it.
Lines 35-36 still state that CI supplies neither backend, so the published build runs likhit alone and reports the other two as unavailable. This new section states the opposite for the deployed site: a replay takes availability from the record and publishes all three columns. A reader who stops at the first statement draws the wrong conclusion about what the published page shows.
Update the earlier paragraph to describe the replay, or add a forward reference to this section there.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/README.md` around lines 100 - 107, Update the earlier Configurations
paragraph to accurately describe the deployed site's replay behavior, including
that availability is read from recorded results and all published columns can be
shown; alternatively, add a clear forward reference to the later replay section.
Keep the CI configuration details consistent with the explanation in the replay
section.
| spend: dict[str, dict[str, int]] = { | ||
| name: {"calls": 0, "tokens": 0} for name in artifact["configurations"] | ||
| } | ||
| for document in artifact["documents"]: | ||
| for run in document["runs"]: | ||
| usage = run.get("ocr_usage") | ||
| if usage and run["config"] in spend: | ||
| spend[run["config"]]["calls"] += usage["calls"] | ||
| spend[run["config"]]["tokens"] += usage["total_tokens"] | ||
|
|
||
| rows = [] | ||
| for name, config in artifact["configurations"].items(): | ||
| counts = tally[name] | ||
| total = sum(counts.values()) | ||
| if config.get("available"): | ||
| note = f"{total} run(s)" | ||
| if config.get("model"): | ||
| note += f" · `{config['model']}`" | ||
| calls = spend[name]["calls"] | ||
| if calls: | ||
| note += f" · {calls} vision call(s), {spend[name]['tokens']:,} tokens" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report measured zero-call OCR usage in the CI summary.
_configuration_rows() only appends OCR usage when calls is non-zero. A configuration with recorded zero calls therefore renders the same note as one with unavailable usage data.
Track whether any ocr_usage record was present. If usage was measured, render 0 vision call(s), 0 tokens. If usage was absent, render an explicit unavailable-usage state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@site/summarize.py` around lines 89 - 109, Update _configuration_rows() to
track whether any ocr_usage record was present for each configuration,
separately from its call count. Render the measured usage note whenever usage
was recorded, including zero calls and tokens; when no usage record exists,
render an explicit unavailable-usage state instead of omitting OCR information.
1 participant
User description
Why
CI cannot measure this benchmark. A runner has no vision backend, so a live build could only ever exercise the no-OCR configuration — 16 runs with 32 skipped — and converting the full corpus three ways takes far longer than a Pages build should. The published page therefore showed a degraded subset of what the benchmark can actually say.
This records a complete local run and replays it. CI now publishes 16 documents / 48 runs with no credentials at all, verified by running the workflow's exact command with the environment stripped.
Recorded results
--write-snapshotrecords each run's status, text, timing, memory and OCR usage intosite/snapshot.json;--snapshotreplays it, converting nothing and calling no backend. Availability comes from the recording rather than the environment — deriving it locally would drop the very OCR columns the snapshot exists to publish.Checks and outcomes are recomputed from the recorded text, so a catalog edit takes effect without re-recording. That is how the
npc-press-noteexpectation below applies to an already-committed snapshot.The property this rests on is that a replay is indistinguishable from the run it recorded. Verified against the real build: identical summary, metrics, checks and artifact hashes, and byte-identical transcripts and diagnostics across all 48 runs. That is why the recording stores the raw stderr rather than the composed diagnostic file — composing it twice would append the error lines a second time. A test covers it, using a fixture that includes a failing run, because with nothing to append the composition is the identity function and the test would not bite.
Provenance is split deliberately:
buildis the commit that published the page,measuredthe commit whose behaviour the numbers describe.measured.staleflags a divergence,missing_runsnames catalog runs the recording predates, and the dashboard renders both above the summary. Run metadata attributes the conversion to the recorded environment, not the publishing one. A live build setsmeasuredto null.summarize.pywrites the numbers, per-configuration outcomes, models and token totals to$GITHUB_STEP_SUMMARY, so a regression shows on the Actions run rather than only in the deployed page.OCR model and tokens
Two things made OCR reporting misleading:
_ocr_usage_recordreturnednullwhen a run made no vision call — which is most runs, since Likhit only calls a model for pages a text layer cannot serve. That made "spent nothing" indistinguishable from "nobody was counting", and the dashboard drew a blank for both. Zero is now a measurement;nullmeans only that the counter was unreachable.The model is now recorded per configuration and shown beside the configuration selector:
The committed recording covers all 16 runs of both OCR backends — 3 with real spend, 13 as measured zeros:
us.anthropic.claude-opus-5qwen2.5vl:7bsite/ocr_usage_proxy.pyis a reference implementation of the counter contract the README documented but nothing shipped: it forwards to any OpenAI-compatible endpoint and accumulates theusageblock each response carries. One per backend, so a hosted run cannot be credited with a locally served model's spend.Documentation
The OCR section of both the README and the landing page now shows the two setups people actually reach for — a vision model on your own machine, and Bedrock behind a translating proxy. Bedrock gets explicit treatment because Likhit speaks only the OpenAI chat API; there is no Bedrock support in the package, so pointing it straight at Bedrock cannot work and the generic "custom base URL" option would lead a reader astray.
Both also record when OCR runs: only for pages a text layer cannot serve, so a configured backend does not mean paying for one per document — 13 of the 16 benchmark documents convert with no vision call. A test pins the README and the page to each other and to
_resolve_ocr_env, so a renamed variable fails rather than shipping.Behaviour notes for the reviewer
npc-press-noteunder offline OCR is intermittently slow, not broken. It exhausted the 2400s budget on one recording and completed in 1073s on the next. Markedknown_issue, so a timeout is reported instead of failing the build while a run that completes is promoted back topasson its own — which is what the committed recording shows. Without this the run would block every Pages deploy.runsarray, which the schema forbids. Latent before this change: any document declaring only OCR runs would have hit it.ocr_usagesemantics changed for consumers.nullnow means only "not counted". The schema already permitted both shapes, soschema_versionstays 1.measuredis added as optional rather than required, so artifacts published before it existed still validate..detail-content's grid rows so the tab bar and body shared a cell, leaving Metadata and Checks unclickable. Playwright surfaced it as an intercepted click; measuring the geometry confirmed it was real. The subtlety is that#document-sourceisdisplay: none, so it takes no row. A test now parses the DOM and asserts one row per in-flow child, catching a row too few, a row too many, and a new child added without one.Verification
uv run ruff check .clean;ruff format --check .clean (63 files)uv run pytest— 321 passed, 5 skipped, 4 xfaileduv run ty check— 3 diagnostics, all pre-existing onmainPR Type
Enhancement, Tests, Documentation
Description
Replay recorded benchmark snapshots
Publish OCR model, token metadata
Add CI benchmark summary
Document OCR setup recipes
Diagram Walkthrough
File Walkthrough
1 files
Add snapshot replay and OCR tests6 files
Implement benchmark snapshot replay recordingAdd OpenAI token counting proxyAdd Actions benchmark summary rendererRender recorded provenance and OCR usageStyle OCR recipes and provenance UIExtend schema with measurement provenance3 files
Add OCR recipes and provenance bannerDocument snapshots and token accountingDocument OCR recipes and benchmark link1 files
Replay snapshots and publish summary1 files
Mark local OCR timeout known issue1 files
Add committed benchmark result snapshot🛠️ Relevant configurations:
These are the relevant configurations for this tool:
[config]
[pr_description]
Summary by CodeRabbit
New Features
Bug Fixes
Documentation