docs(tuning): fix env contract, rewrite stale README, document precedence; honest auto-mode message#1178
docs(tuning): fix env contract, rewrite stale README, document precedence; honest auto-mode message#1178joeharris76 wants to merge 1 commit into
Conversation
…ence; honest auto-mode message Re-verified the 2026-07-12 tuning review findings (R5) against HEAD (post-#1172 resolver hints/keyword-first fixes) before implementing: - w1: BENCHBOX_TUNING_ENABLED set config key tuning.enabled, which nothing at runtime read except a unit test. Removed both env-var mapping mechanisms (module-level env_mappings dict and the ConfigManager default's environment_overrides entry) from benchbox/cli/config.py, the dead "enabled" default key, and the false "activates tuned runs in CI" doc claims in docs/usage/configuration.md and docs/reference/cli/configuration.md. BENCHBOX_TUNING_CONFIG and BENCHBOX_TUNING_PATH behavior is unchanged. Added a CHANGELOG entry. - w2: rewrote examples/tunings/README.md - the real flag is --tuning (not --tuning-config), the named example scripts don't exist (pointed at the real examples/unified_runner.py and examples/features/tuning_comparison.py instead), file layout matches examples/tunings/<platform>/<benchmark>_tuned.yaml, auto-discovery via --tuning tuned is the primary UX, and dropped the unqualified "2-10x" performance claim. - w3: documented the actual --tuning resolution order in docs/reference/cli/tuning.md (default notuning -> keyword check before path check -> tuned's default_config_file/BENCHBOX_TUNING_CONFIG beats discovery -> BENCHBOX_TUNING_PATH -> cwd-relative examples/ -> cwd -> fallback with warning), the cwd-relative caveat for installed-package users, the tuning list/show/platforms subcommands, and the correct --output default (<platform>_tuning.yaml / DataFrame's <platform>_<profile>_tuning.yaml, not tuning_config.yaml). - w4: --tuning auto now warns explicitly on SQL platforms that smart defaults are DataFrame-only today and the run proceeds with a basic config, instead of silently building an untuned config while claiming smart defaults were applied. Message change only, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QTnjb2i44JUL71kmu9sk1t
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e165d220
ℹ️ 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".
| "[yellow]Warning: --tuning auto smart defaults are DataFrame-only today; " | ||
| "this SQL run proceeds with a basic (untuned) configuration.[/yellow]" |
There was a problem hiding this comment.
When --tuning auto is used on SQL platforms, this branch sets s.loaded_unified_config = UnifiedTuningConfiguration() while s.tuning_enabled remains true; that default enables PK/FK/unique/check constraints and is passed to adapters as tuning. The new warning says the run is “untuned”, so users comparing against --tuning notuning can unknowingly run a constraints-enabled configuration. Please either disable all constraints here or describe this as a constraints-only/basic configuration rather than untuned.
Useful? React with 👍 / 👎.
2 participants
Pull Request
Description
Implements TODO
tuning-docs-env-contract-fixes-20260712(review finding R5 — the broken docs/env contract):BENCHBOX_TUNING_ENABLEDdeleted — it set a config key nothing at runtime reads, while docs claimed it "activates tuned runs in CI". Both override mechanisms removed, doc claims fixed, CHANGELOG entry added.examples/tunings/README.mdrewritten against reality — the documented--tuning-configflag and example scripts never existed; now documents--tuning, the real template layout, auto-discovery as the primary UX,BENCHBOX_TUNING_PATH, and drops the unqualified "2-10x" claim.docs/reference/cli/tuning.md) — the full resolution order (defaultnotuning→ keywords → explicit path →default_config_file/BENCHBOX_TUNING_CONFIG→BENCHBOX_TUNING_PATH→ cwd-relative discovery → fallback), every rule re-verified against current code; the cwd-relative discovery caveat for installed-package users;tuning list/show/platformsdocumented (including thattuning listdoes not honorBENCHBOX_TUNING_PATH); corrected--outputdefault.--tuning autohonesty — SQL runs now get an explicit warning that smart defaults are DataFrame-only and the run proceeds with a basic config (message-only; behavior change stays gated on the vocab TODO).Moves the TODO to DONE.
Type of Change
Testing
make lintandmake docs-validatepassing; repo-wide grep for the deleted env var returns emptyPublic Contract Check
Documentation
Code Quality
BENCHBOX_TUNING_CONFIG/BENCHBOX_TUNING_PATHbehavior untouched (verified).Artifact Hygiene
Notes
Scope note:
CHANGELOG.mdis one file beyond the TODO'sonly_modify— the TODO's w1 explicitly calls for a release note.Part of the tuning remediation batch (#1161).
🤖 Generated with Claude Code
https://claude.ai/code/session_01QTnjb2i44JUL71kmu9sk1t
Generated by Claude Code