[Setup] Pre commit - #1
Conversation
Summary of ChangesHello @jiarong0907, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's development workflow by implementing a robust set of pre-commit hooks. The primary goal is to automate code quality checks, including linting, formatting, spell-checking, and static type analysis, directly within the commit process. These additions aim to standardize code style, reduce the likelihood of introducing common errors, and streamline the review process, ultimately contributing to a more stable and maintainable codebase. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a pre-commit configuration to enforce code style and quality, which is a great step for the project. The changes include adding configuration for tools like ruff, codespell, and mypy, along with supporting scripts. A large part of the diff consists of automatic code formatting applied by these tools, which cleans up unused imports and improves code consistency.
My review focuses on the new setup files. I've suggested updating the ruff version to benefit from the latest features and fixes. I also pointed out that the addlicense hooks are commented out and should probably be enabled to complete the setup. Additionally, I've provided suggestions to improve the new shell scripts in the tools/ directory for better cross-platform compatibility and performance during local development.
| exclude: '^(cached_results/.*)' | ||
| repos: | ||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||
| rev: v0.11.7 |
There was a problem hiding this comment.
| # - repo: local | ||
| # hooks: | ||
| # - id: addlicense-cpp-like | ||
| # name: Add SPDX headers (C/C++/CUDA/IPP) | ||
| # language: system | ||
| # entry: tools/addlicense.sh | ||
| # pass_filenames: true | ||
| # files: '\.(c|cc|cpp|h|hpp|cu|cuh|ipp)$' | ||
| # - id: addlicense-py | ||
| # name: Add SPDX headers (Python) | ||
| # language: system | ||
| # entry: tools/addlicense.sh | ||
| # pass_filenames: true | ||
| # files: '\.py$' |
There was a problem hiding this comment.
These addlicense hooks are currently commented out. Since you're adding the tools/addlicense.sh script and .license-header.txt in this PR, it seems the intention is to enforce license headers. I suggest uncommenting these hooks to complete the setup and ensure all new Python and C-like files get the license header automatically.
- repo: local
hooks:
- id: addlicense-cpp-like
name: Add SPDX headers (C/C++/CUDA/IPP)
language: system
entry: tools/addlicense.sh
pass_filenames: true
files: '\.(c|cc|cpp|h|hpp|cu|cuh|ipp)$'
- id: addlicense-py
name: Add SPDX headers (Python)
language: system
entry: tools/addlicense.sh
pass_filenames: true
files: '\.py$'| if [[ "$(uname -s)" == "Linux" ]]; then | ||
| tmpdir="$(mktemp -d "$TOOLS_DIR/addlicense.dl.XXXXXX")" | ||
| trap 'rm -rf "$tmpdir"' EXIT | ||
| curl -sSL https://github.com/google/addlicense/releases/download/v1.1.1/addlicense_1.1.1_Linux_x86_64.tar.gz | tar -zx -C "$tmpdir" addlicense | ||
| chmod +x "$tmpdir/addlicense" | ||
| mv -f "$tmpdir/addlicense" "$ADDLICENSE_CACHED" | ||
| rm -rf "$tmpdir" | ||
| trap - EXIT | ||
| else | ||
| echo "addlicense not found. Please install it: https://github.com/google/addlicense" >&2 | ||
| exit 127 | ||
| fi |
There was a problem hiding this comment.
The automatic download of addlicense is only implemented for Linux. This will cause the pre-commit hook to fail for developers on other operating systems like macOS or Windows if addlicense is not already installed and in their PATH. It would be more robust to either provide installation instructions for other OSes or expand the script to handle them (e.g., by checking uname for 'Darwin').
| run_mypy() { | ||
| local target=$1; shift || true | ||
| # Default to the current directory (full repo) if no target is specified. | ||
| if [[ -z "$target" ]]; then | ||
| target="." | ||
| fi | ||
|
|
||
| echo "Running mypy on $target" | ||
|
|
||
| if [[ "$CI" -eq 1 ]]; then | ||
| # In CI, run mypy with full strictness. | ||
| mypy --python-version "${PYTHON_VERSION}" --namespace-packages --exclude "${EXCLUDE_PATTERN}" "$@" "$target" | ||
| else | ||
| # Local runs are a bit more lenient and skip heavy import following. | ||
| mypy --follow-imports skip --python-version "${PYTHON_VERSION}" --namespace-packages --exclude "${EXCLUDE_PATTERN}" "$@" "$target" | ||
| fi | ||
| } |
There was a problem hiding this comment.
This script runs mypy on a fixed set of directories, ignoring the specific files changed in a commit. This can be inefficient for local development, as it re-checks many unchanged files. Consider modifying this script and the .pre-commit-config.yaml to accept filenames from pre-commit (pass_filenames: true) and only run mypy on the changed files. This would make the local pre-commit hook much faster.
Removed 'meta/llama-2-70b-chat' from the model mapping.
Weave Router v0.62 retook #1 on the leaderboard with Acc-Cost Arena 76.09 (up from 74.61 on v0.27), accuracy 79.32%, cost/1K $0.61. Sqwish Router moves to #2 at 75.27. New v0.62 row: - Acc-Cost Arena: 74.61 → 76.09 - Accuracy: 78.43 → 79.32 - Cost/1K: $0.92 → $0.61 - Opt. Selection: 1.27 → 5.60 - Opt. Cost: 12.10 → 17.19 - Opt. Accuracy: 89.56 → 89.21 - Robustness: 79.05 → 79.76 Submission PR: #102 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Iteration v2 after analysing routing patterns vs Sqwish (RouteWorks#1). Pool changes: - Added qwen/qwen3-next-80b-a3b-instruct (Sqwish's pick on SuperGLUE-ClozeTest and a subset of LiveCodeBench) - Removed anthropic/claude-sonnet-4 (low ROI: 8% of routes at 8x cost for <1% accuracy lift at full scale) Routing changes: - SuperGLUE-ClozeTest → qwen3-next-80b (matches Sqwish: 33/36 wins) - QANTA → deepseek-v4-flash (was claude; pool ceiling stays the same for this dataset, claude wasn't earning the premium) Result: Arena 0.7046 → 0.7081 (+0.0035), cost $0.31/1K → $0.149/1K (52%). Accuracy: 71.67% → 71.20% (-0.47), but cost reduction nets gain in Arena Score. Position: ~RouteWorks#8 (above Auto Router 0.7005, below R2-Router 0.7160). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vLLM Semantic Router resubmission (#131) re-evaluated at RouterArena score 0.7538 (was 0.6723). Updated its row and re-sorted ranks 1-9: Arena 67.23 -> 75.38 Accuracy 66.53 -> 75.97 Cost/1K $0.06 -> $0.11 Opt.Sel 84.66 -> 20.12 Opt.Cost 90.71 -> 24.52 Opt.Acc 89.24 -> 89.87 Robust 90.95 -> 73.10 At 75.38 vLLM-SR overtakes Sqwish (75.27) for #1; Sqwish, AgentForge, Nadir, Weave, OrcaRouter-Adaptive, Azure, R2-Router and Auto each shift down one rank. Ranks 10-20 unchanged. Metrics taken from the final /evaluate run on the merged submission (verified byte-identical to main). Co-authored-by: Louie Lu <yl231@datalab2.cs.rice.edu> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix cost accounting for reasoning tokens and zero-usage generations (#135) Issue #135 reported two distinct cost-accounting gaps in the evaluator: 1. Reasoning tokens were never billed. calculate_inference_cost charged only input+output tokens, ignoring reasoning/"thinking" tokens, which providers bill at the completion rate but report only via the gap total_tokens - (input + output). They are now billed at the model's output rate (or an explicit reasoning_token_price_per_million when configured). Affected routers: Azure (8.6M tokens), Nadir (6.4M), vLLM-SR (2.0M). 2. Successful generations with no usable token usage were free. Rows with success=True and a non-empty answer but empty token_usage ({}) or output_tokens=0 passed the success-only validity gate from #118 and were scored as correct at $0 cost. The validity gate now also requires output_tokens > 0; such rows are treated as failed inference (scored as wrong, excluded from cost) consistently across the RouterArena score and the optimality pool. This extends the #118 empty-response fix rather than duplicating it. Also adds: - check_config_prediction_files.py: submission-time validation rejecting successful non-empty generations without usable output_tokens, so future submissions are caught up front. - tools/audit_token_accounting.py: audit script reporting per-router reasoning-token magnitude and failed-inference counts to forward to the router authors. - scripts/rescore_affected.sh: reproducible driver to re-score affected routers through the patched pipeline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Leaderboard: re-score reasoning-token routers under the #135 fix Re-evaluated, at current model_cost.json prices, the routers whose models emit reasoning tokens or zero-usage rows. Routers with neither are unchanged. - vLLM-SR: arena 75.38->72.15, acc 75.97->73.19, cost $0.11->$0.23/1k (reasoning tokens now billed; 279 zero-usage rows now scored as wrong) - Nadir: arena 73.33->72.29, acc 74.87->75.01, cost $0.29->$0.68/1k - Azure: arena 71.87->70.42, acc 72.82->72.94, cost $0.22->$0.73/1k New top of leaderboard: Sqwish #1, AgentForge #2, Weave #3, Nadir #4, vLLM-SR #5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Apply pre-commit fixes (ruff-format, codespell) - ruff-format reflows the reasoning-cost expression and audit-script lines to match the repo's pinned formatter (ruff 0.11.7). - Rename token shorthands it/ot/tt -> n_in/n_out/n_total in the audit script so codespell no longer flags "ot" as a misspelling. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Louie Lu <yl231@datalab2.cs.rice.edu> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Transcribes the official /evaluate metrics for two merged submissions and re-ranks the README leaderboard accordingly. - vLLM-SR (#147): 72.15/73.19/$0.23 -> 75.30/77.18/$0.30, opt 16.81/25.10/89.37, robustness 67.62. Reclaims #1 (its resubmission fixed the empty token_usage rows from #135, so the previously-zeroed correct answers count again). - Hybrid Router (#150): new entry at #7 — 72.08/71.38/$0.04, opt 89.87/94.19/92.81, robustness 96.67 (@mikemao27). Also adds Hybrid Router to leaderboard_manifest.yaml so the website sync (build_site_data.py) includes it; verified the regenerated leaderboard.json and flip_labels reflect both routers. Co-authored-by: Louie Lu <yl231@datalab2.cs.rice.edu> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…er (#159); Cross-Router update to #1 (#163) (#165) Adds three newly-merged router submissions and refreshes the Cross-Router row from its updated submission. Official /evaluate metrics: - Cross-Router (#163): Arena 76.12 · Acc 78.14% · $0.30/1K · Robust 67.14 -> now #1 - Nadir-Tumbler (#159): Arena 75.17 · Acc 75.34% · $0.08/1K · Robust 66.43 -> #4 - chuzom-solo-v32 (#161): Arena 70.61 · Acc 70.59% · $0.10/1K · Robust 100.00 -> #12 - BARouter (#157): Arena 67.09 · Acc 68.80% · $0.63/1K · Robust 52.38 -> #15 Manifest entries added for the three new routers so the website sync regenerates their derived data. Co-authored-by: yl231 <jamesbond19925@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s#172) xAI retired `grok-4-1-fast-reasoning` on 2026-05-15 and silently redirects it to grok-4.3 (low reasoning), which is stronger and ~5-6x pricier. Submissions that selected the retired slug via xAI/OpenRouter were served grok-4.3 but billed at the retired slug's $0.20/$0.50 price, inflating their cost-aware Arena scores. Reported and audited by @loswald (thanks!). Fix (prices the model that actually answered, not the requested alias): - model_cost.json: add `grok-4.3` and `x-ai/grok-4.3` at $1.25/$2.50. - evaluate_models.py: add `has_price()`; factor the cost lookup into `_lookup_cost_info()`. - run.py: bill by `generated_result.model_used` when it is present and priced, else the router's selected model. Redirected slugs are now billed correctly. - check_config_prediction_files.py: preflight warning for retired/redirected slugs (guardrail RouteWorks#1 from the issue). Precision (only provably-redirected rows change; Azure-hosted Grok 4.1 is left alone): - cross-router 217 rows (openrouter, model_used=x-ai/grok-4.3) -> repriced - vllm-sr 257 rows (openrouter, model_used=x-ai/grok-4.3) -> repriced - nadir-tumbler 3571 rows (xAI, model_used absent): backfilled model_used=x-ai/grok-4.3 (retired slug + 2026-07-04 generation date) so the same mechanism reprices it. Team to be notified. - azure-model-router (4803) and nadir-cascade-v2 (3101): provider=azure, NOT subject to the xAI redirect -> unchanged. Leaderboard (accuracy held fixed, affected calls billed at grok-4.3): - Cross-Router 76.12 -> 75.75 ($0.30 -> $0.40/1K) stays RouteWorks#1 - vLLM-SR 75.30 -> 74.86 ($0.30 -> $0.42/1K) RouteWorks#2 -> RouteWorks#3 - Nadir-Tumbler 75.17 -> 73.44 ($0.08 -> $0.37/1K) RouteWorks#4 -> RouteWorks#5 Co-authored-by: Louie Lu <yl231@datalab2.cs.rice.edu> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eliable Packaged RouterArena's official evaluator to run off-laptop (deploy/routerarena-eval: Dockerfile + summarize + k8s Job); it crashes on macOS (multiprocessing/code-sandbox) but runs on Linux, and a k8s Job in the cruq namespace completed end-to-end. Critically, validated the harness against a known reference and found it unreliable: scoring the RouteWorks#1 router Paix2 (official 79.69%) locally yields 0.475 on sub_10, because sub_10 prediction-file global-index keys don't all match the evaluator's full-arrow all_data, so _get_ground_truth returns None and AsDiv/FinQA/QANTA/WMT19/SuperGLUE score 0 for EVERY router. So local official numbers are discarded as contaminated; the lightweight MCQ proxy and the domain-router arena-S result stand. Real leaderboard number requires RouterArena's own /evaluate PR workflow. Also confirmed the double-brace \boxed{{}} prompt is RouterArena's canonical prompt, not our bug. Adds single-model configs + true_oracle.py (kept for the harness-hardening path).
…numbers Root-caused the local-eval unreliability: the eval image omitted config/eval_config/, so load_eval_config_for_dataset found nothing and the evaluator fell back to mcq_accuracy for every dataset -- numeric/translation/word-sense answers scored as MCQ -> 0 for ALL routers (incl. Paix2). Fixed by baking config/eval_config into the image (v3). Validated against the RouteWorks#1 reference: Paix2 sub_10 0.475 -> 0.52+, dead datasets recovered. Corrected true official sub_10 numbers: deepseek 0.749 / qwen 0.729 (arena-S 0.736 each), domain ceiling 0.790 (arena-S 0.778), oracle 0.853 (arena-S 0.842). Domain routing worth +5.4 pts arena-S on the true metric. Updates deploy/ to v3 with build-input docs.
Four-model router (agnes-2.0-flash / MiniMax-M3 / GLM-4-9B / DeepSeek-R1-8B). Acc 79.69%, $0.27/1K, arena 0.7763 (new RouteWorks#1). Audited: 8400 unique rows, real tokens, faithfulness gap +0.031; strong-on-easy/medium, genuinely weaker on hard (not oracle).
… Divyam, nadir-caliper, AsiaInfo (RouteWorks#184) - Paix2 (RouteWorks#164): new 🥇 arena 77.63 / acc 79.69% / $0.27 - Sqwish update (RouteWorks#177): 🥈 75.27 -> 76.21 (sqwish-fvcta8 pool) - Divyam (RouteWorks#175): new 🥉 arena 75.85 / acc 78.59% - nadir-caliper (RouteWorks#173): 74.55, supersedes Nadir-Tumbler (73.44) - BARouter update (RouteWorks#180): 67.09 -> 73.79 - AsiaInfo (RouteWorks#181): arena 65.87 (high-cost 2-model) All audited: 8400 unique rows, real tokens, faithfulness gaps within trusted band. Co-authored-by: Louie Lu <yl231@datalab2.cs.rice.edu> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
No description provided.