Codebase improvements: tests, lint/format, refactoring, and documentation cleanup#11
Merged
Merged
Conversation
- Remove dead 'Check solver availability' step from validate.yml: it imported a non-existent scripts.utils.solver_validation module and masked all failures with '|| echo', so it never validated anything. Actual solver validation is done by 'python main.py --validate'. - Replace 9 bare 'except:' clauses with specific exception types and debug/warning logging (cvxpy_runner.py capability probes and version lookup, runner.py problem config fallback). - Drop pkg_resources fallback in _get_package_version (project requires Python 3.12+, importlib.metadata is always available) and remove a no-op try/except around a dict assignment. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
- Introduce tests/ with unit tests for the SDPA and SeDuMi data loaders, SolverResult validation and factory methods, DatabaseManager storage and latest-result queries, and ProblemInterface registry handling (74 tests, all using synthetic fixtures generated in tmp_path). - Add integration checks validating config/problem_registry.yaml integrity; file-existence check skips when problem submodules are not checked out. - Add a regression test asserting solver name consistency between PythonSolverManager and PythonProcessInterface configs. - Fix DatabaseManager.ensure_schema to resolve schema.sql relative to the module instead of the current working directory. - Add pyproject.toml with pytest configuration; dev tools (pytest, ruff) are installed in CI rather than via requirements.txt, which remains the reproducible benchmark runtime definition. - Run the test suite in validate.yml before system validation. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
- Configure ruff (lint + format) in pyproject.toml: rules E, F, W, I, B with E501 (long lines) and E402 (imports after sys.path bootstrap) ignored; problems/ and docs/ excluded. - Apply safe autofixes: import sorting, unused imports, f-strings without placeholders, missing trailing newlines. - Manual fixes: exception chaining (raise ... from), unused variables and loop variables, ambiguous variable name, explicit zip strict=, noqa annotations for intentional availability-check imports. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
Pure formatting commit: ruff format plus trailing-whitespace removal inside docstrings and multi-line strings. No logic changes; test suite and system validation pass unchanged. Also add a lint/format check step to the validation workflow. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
PYTHON_SOLVER_CONFIGS was duplicated between python_solver_runner.py and python_process_interface.py, so adding a solver required editing both files and the display names had drifted apart. - Add scripts/solvers/python/solver_configs.py as the single source of truth. It holds only declarative data (no heavy imports) so the main process can import it without loading cvxpy/scipy, preserving the point of subprocess isolation. - python_solver_runner.py resolves solver classes from the declarative 'runner' key via RUNNER_CLASSES; python_process_interface.py now references the shared dict. - Update test_solver_configs.py to assert both classes share the same config object and all runners are resolvable. Verified with the test suite, 'main.py --validate', and a one-problem subprocess dry-run benchmark. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
html_generator.py had grown to 2,034 lines with four report generators and their shared helpers in a single class. Split it as a pure move (method bodies unchanged): - report_base.py: ReportGeneratorBase with shared state and the common helpers (_analyze_multiple_environments, _generate_environment_section, _get_platform_info, _get_common_css) - overview_report.py, results_matrix_report.py, raw_data_report.py, data_index_report.py: one generator class per report type - html_generator.py: HTMLGenerator kept as a ~130-line facade delegating to the four generators; public API (generate_all_reports, generate_overview, ...) is unchanged Verified by golden-file comparison: reports generated from the same seeded database before and after the split are byte-identical except for generation timestamps. Test suite and 'main.py --validate' pass. https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
Fix references that no longer match the code: - Solver addition now points to scripts/solvers/python/solver_configs.py (single source of truth) instead of the non-existent python_interface.py / config/solver_registry.yaml (detail_design.md, CONFIGURATION.md, LOCAL_DEVELOPMENT_GUIDE.md, README.md, CLAUDE.md) - Directory layout reflects the split reporting modules, solver_configs.py, tests/, and pyproject.toml; removed the non-existent resource_limits.py - Testing sections describe the real pytest suite (tests/unit, tests/integration, pyproject.toml config) instead of fictional file names - requirements/python.txt and requirements/base.txt references replaced with the single requirements.txt - Rewrote .github/workflows/README.md: it described a benchmark.yml workflow that does not exist; now documents the real validate.yml and deploy.yml - Replaced the unsupported --problem-set flag with --library_names in command examples; added ruff/pytest notes to coding standards https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
Apply the policy that documentation should not repeat what the code already says: - Replace full code listings (SolverResult dataclass, ProblemData, SQL schema, MATLAB/Python style examples, interface signatures) with short role descriptions plus source file references - Remove historical/status content (CLAUDE.md development status, basic_design.md completed-features list and solver coverage table) - Remove sections describing practices that do not exist in this repo (release management, GitHub Actions caching for a benchmark workflow) - Consolidate duplicated content: project structure now lives only in detail_design.md; README design philosophy compressed to bullets with a link to basic_design.md - Convert the subprocess error-flow ASCII art to a short bullet list https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
- Fold MANUAL_TRIGGER_GUIDE.md into GITHUB_ACTIONS_SETUP.md and delete it - Rewrite GITHUB_ACTIONS_SETUP.md against the real workflows: it referenced non-existent deploy-pages.yml / pr-preview.yml files, the wrong report path (docs/ instead of docs/pages/), and claimed CI runs no Python (validate.yml runs ruff/pytest/validation) - Fix dead links in EXTERNAL_LIBRARIES.md (tasks.md, merged guide) and the pr-preview.yml reference in PR_PREVIEW_GUIDE.md - Replace the docs/guides/README.md status page (which tracked known inaccuracies, now resolved) with a simple guide index https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
CLAUDE.md is loaded into every AI session, so verbosity is most expensive here. Reduce 133 lines to 97 without dropping any constraint: - Merge the duplicated read-the-links warnings into one sentence and drop the auto-compact re-read instruction (no longer useful in current agent environments) - Collapse the four restatements of the single-requirements.txt rule into one bullet, and attach the rationale to each constraint so the reasoning travels with the rule - Remove emphasis inflation (emoji headers, all-caps warnings) so the genuinely critical section stands out https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
Fix statements that no longer match the code: - Problem counts: registry currently holds 123 problems (DIMACS 37, SDPLIB 86), not '47 + 92+ = 139+'. Replace hard-coded counts with an approximate figure plus a pointer to config/problem_registry.yaml as the authoritative list, so the docs do not drift again - Problem registry schema: file_type is 'mat' | 'dat-s' only and library_name is 'DIMACS' | 'SDPLIB' (mps/qps/internal are not implemented) - Resource control: timeouts are enforced; the subprocess memory-limit mechanism exists but is not currently passed by the process interfaces, so stop claiming unified memory limits - Reports are static HTML with inline CSS, not Bootstrap 5 + Chart.js (detail_design.md and CLAUDE.md) - Drop the self-congratulatory closing line and remaining emoji from detail_design.md https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
- Drop generated-with footer and Co-Authored-By line from the Git commit format in conventions.md https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
📊 Preview Reports Ready!Your preview has been deployed and is available at: Preview Details
Available Reports
Notes
Preview deployment powered by GitHub Actions |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A series of codebase quality improvements across CI, code structure, and documentation.
Code quality & tooling
|| echopatterns) and fix bareexceptclauses so validation failures actually fail CItests/) and run it in CIruff formatacross the codebase (bulk format commit listed in.git-blame-ignore-revs)Refactoring
scripts/solvers/python/solver_configs.py)html_generator.pyinto per-report generator modulesDocumentation
https://claude.ai/code/session_012qzeYBSL3NxES4KgLaR79Y
Generated by Claude Code