Skip to content

Make pre_push_check able to pass, and fix the defects it was hiding - #138

Merged
jeremymanning merged 3 commits into
masterfrom
fix/133-135-tooling
Aug 18, 2026
Merged

Make pre_push_check able to pass, and fix the defects it was hiding#138
jeremymanning merged 3 commits into
masterfrom
fix/133-135-tooling

Conversation

@jeremymanning

@jeremymanning jeremymanning commented Aug 18, 2026

Copy link
Copy Markdown
Member

Closes #133. Also confirms #135 is resolved.

#133 — pre_push_check.py could never pass

It promises "run this before pushing to ensure GitHub Actions won't fail",
and failed for two independent reasons.

It ran the wrong tools. Bare black, flake8, mypy, pytest resolve
against PATH. Here that found Anaconda's mypy 1.19 rather than the project's
2.3, which reported 27 Library stubs not installed errors for stubs
pyproject.toml does declare — so the script failed while CI, which installs
the dev extra, passed. Tools now run as <this interpreter> -m <tool>.

Its flake8 step reported 91 findings. Four were real defects:

Where Defect
test_direct_gpu_detection.py a remote program wrapped in an outer f-string, so {torch.__version__}, {i}, {props.name}, {e} interpolated locally → NameError before anything was sent
validate_container_registry.py json.loads with no module-level import (the only import json is inside an embedded script) → NameError, swallowed by a bare except that blamed the output
debug_slurm_output_location.py \$ inside an f-string — not a Python escape, warns on 3.12
two more a W504 continuation and four trailing spaces inside embedded scripts

The other 74 were all E402 from a single deliberate pattern: 30 standalone
validation and debug scripts that adjust sys.path, or set an environment
variable clustrix reads at import time, before importing the package they
exercise. That ordering is the point of them running standalone, so it is
recorded once in .flake8 rather than as 74 noqa comments.

🎉 All checks passed on attempt 1! Safe to push.

#135 — fast_ci.yml is invalid YAML and has never run a job

Fixed by #137. The workflow parses and runs five jobs:

success  2094b93  jobs=5     (was: failure, jobs=0)

Also

Cleaned the lint in scripts/, which nothing checks and which fixing the
above surfaced — unused imports and f-strings with no placeholders.

361 tests pass; black, flake8 and mypy clean on clustrix/, tests/ and
scripts/.

🤖 Generated with Claude Code

https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

jeremymanning and others added 3 commits August 18, 2026 19:05
@jeremymanning @claude
Make pre_push_check able to pass, and fix the defects it was hiding
f5f3db6 
Issue #133: the script promises "run this before pushing to ensure GitHub
Actions won't fail" and could not pass. Two independent reasons.

It shelled out to bare `black`, `flake8`, `mypy` and `pytest`, so it ran
whatever was first on PATH. Here that was Anaconda's mypy 1.19 rather than the
project's 2.3, which reported 27 "Library stubs not installed" errors for
stubs pyproject does declare -- the script failed while CI, which installs the
dev extra, passed. Tools now run as `<this interpreter> -m <tool>`, so the
script checks the environment it is running in.

Its flake8 step reported 91 findings. Four were real defects, not noise:

* tests/integration/test_direct_gpu_detection.py wrapped a remote Python
  program in an *outer f-string*, so `{torch.__version__}`, `{i}`,
  `{props.name}` and `{e}` were interpolated locally and the test raised
  NameError before it could send anything. The program is now a plain string
  and reads CUDA_VISIBLE_DEVICES on the far side, which is where it lives.
* validate_container_registry.py calls json.loads with no module-level import
  -- the only `import json` is inside an embedded container script -- so the
  call raised NameError, swallowed by a bare except that reported it as
  unparseable output.
* debug_slurm_output_location.py wrote `\$` inside an f-string, which is not a
  Python escape and warns on 3.12. Doubled, so the shell still receives `\$`.
* A W504 continuation and four trailing spaces inside embedded scripts.

The remaining 74 were all E402 from one deliberate pattern: 30 standalone
validation and debug scripts that adjust sys.path, or set an environment
variable clustrix reads at import time, before importing the package they
exercise. That ordering is the point of them running standalone, so it is
recorded once in .flake8 rather than as 74 noqa comments.

Also cleaned the lint in scripts/ that nothing checks, since fixing the above
surfaced it: unused imports and f-strings with no placeholders.

    🎉 All checks passed on attempt 1! Safe to push.

Issue #135 (fast_ci.yml is invalid YAML, 0 jobs) was fixed by PR #137: the
workflow now parses and runs five jobs, all green.

361 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
@jeremymanning @claude
Stop the coverage-badge step failing every push to master
c6491eb 
Tests passed on the merge commit and master still went red. The failure was
after them, in a step that runs only on master and therefore never on a pull
request:

    📊 Current coverage: 17%
    ❌ No coverage badge found to update
    ##[error]Process completed with exit code 1

There is no coverage badge to find, and that is deliberate: the badge that
used to be there showed one of several conflicting numbers, none measured
reproducibly, and it was removed rather than left asserting something untrue
(#115). The updater was never told, so it treated the README agreeing with
that decision as an error.

A missing badge is now a no-op that reports the measured figure and says how
to opt in, rather than a failure. The follow-on commit step only pushes when
there is something to push, instead of asking the runner to resolve a branch
for an empty commit.

Verified against the real README: exit code 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
@jeremymanning @claude
Keep the docs environment off the vulnerable black range
bf524a4 
The dependabot alert stayed open after pyproject and setup.py were pinned,
because it points at docs/requirements.txt, where black is not named at all --
it arrives transitively through the Jupyter stack, so nothing constrained it.

A floor constraint there keeps that environment off the affected range
(24.3.0 up to 26.3.1, arbitrary file writes via the cache file name) without
claiming black is a documentation dependency.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
@jeremymanning
jeremymanning merged commit 0ca28fa into master Aug 18, 2026
18 checks passed
This was referenced Aug 18, 2026
Closed
Closed
jeremymanning added a commit that referenced this pull request Aug 19, 2026
@jeremymanning @claude
Issue #147: make the real-world runner able to fail
9b90eaa 
main() dropped every runner.run_*_tests() return value and never called
sys.exit, so the script exited 0 no matter what. The pre-push hook guards
each category with `if ! python scripts/run_real_world_tests.py --<cat>`,
which therefore could never fire: four categories printed "failed" and the
hook still announced "All real-world tests passed!" and allowed the push.

Same class as the flake8 --exit-zero and mypy continue-on-error steps
fixed in #138 -- a check that reports problems but cannot fail.

Also fixes the second half of #147: the failure message printed only
result.stdout, which was empty in all four observed failures because a
pytest collection error goes to stderr. _report_failure now prints the
exit code, stdout, stderr, and says so explicitly when there was no
output at all.

Verified by appending a deliberately failing test to
tests/real_world/test_filesystem_real.py and running the script:

    EXIT CODE: 1
    ❌ Filesystem tests failed (exit 1)
        assert False, "deliberate failure to verify exit-code propagation"
    E   AssertionError: deliberate failure to verify exit-code propagation

and, with that test removed, the same command exits 0. Before this change
the failing case also exited 0 with an empty message body.

Also drops the removed backends from two scripts: the kubernetes tutorial
entry in check_docs_examples' _SECTION_BOUNDS (that page is deleted) and
the aws/azure/gcp/lambda_cloud entries in the credential display names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
@jeremymanning jeremymanning mentioned this pull request Aug 19, 2026
Merged
jeremymanning added a commit that referenced this pull request Aug 19, 2026
@jeremymanning @claude
Remove every backend that has never been run against real hardware (#149
f78d153 
)

* Notes: session state for the backend removal

* WIP: backend removal, incomplete -- DO NOT MERGE

Checkpoint of two parallel removal agents that were stopped mid-flight so
the machine could be suspended. The tree in this commit DOES NOT IMPORT:

    ModuleNotFoundError: No module named 'clustrix.executor_kubernetes'

clustrix/executor.py still imports KubernetesJobManager, and utils.py still
has the PBS/SGE script generators, because the code agent was stopped just
as it reached utils.py. Committed only so the work survives the suspend.

Done so far: 95 files deleted (cloud_providers/, cost_providers/,
pricing_clients/, kubernetes/, executor_cloud.py, executor_kubernetes.py,
cloud_provider_manager.py, cost_monitoring.py, auto_install.py, 9 notebooks,
kubernetes/pbs tutorials, cost_monitoring API page); 7 files partially
edited.

--no-verify is deliberate: the pre-commit hook runs black/flake8/mypy, and
a tree that cannot import cannot pass them. The next commit on this branch
must pass the full gate.

Resume instructions: notes/2026-08-19-backend-removal-session.md

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: suspend checkpoint and resume instructions

Records that 07db2e6 is a non-importable WIP tip, exactly what remains in
each lane, and the four-check gate the next commit has to pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: record the two outstanding CI facts at the checkpoint

The master Tests run was still in flight at suspend, and an older Real World
Tests failure on 0ca28fa has not been examined.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: record issue #147, the pre-push hook that cannot fail

Pushing the WIP branch surfaced it: four real-world categories reported
failure and the hook announced success. run_real_world_tests.py drops every
result and exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Docs: remove PBS/SGE/Kubernetes/cloud backend references from Sphinx sources

Every remaining reference to a removed backend in docs/source now points at
the :ref:`removed-backends` note in limitations.rst, which names the tracking
issues (#140-#146) and says the backends are planned for a future release
rather than currently supported.

Also drops the broken :doc:`cost_monitoring` cross-references left behind by
the deletion of docs/source/api/cost_monitoring.rst.

* Remove unverified backends from the legacy notebook widget

Drops the Kubernetes/AWS/Azure/GCP/Lambda UI sections, credential fields,
region/instance-type population and connectivity tests, the cost-monitoring
checkbox, and the pbs/sge branches. The cluster-type dropdown now offers
exactly local, ssh, slurm and huggingface, and the HuggingFace section
targets HF Jobs rather than Spaces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Remove the Kubernetes section from the modern notebook widget

Drops the k8s_* controls, their BACKEND_ONLY_FIELDS entry and their
WIDGET_MANAGED_FIELDS entries, and narrows the pbs/sge conditionals to
ssh/slurm. The cluster-type dropdown is driven by SUPPORTED_CLUSTER_TYPES,
so it follows config.py without a change here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Remove unverified backends from the core execution path

The package imports again. Removed from the modules this commit owns:

- executor.py: the KubernetesJobManager and CloudJobManager re-exports,
  which pointed at modules already deleted.
- config.py: every k8s_*, aws_*, azure_*, gcp_*, lambda_*, cloud_* and
  cost_monitoring field (79 lines); the auto_install cloud-dependency
  hook in __post_init__ and in configure(), along with configure()'s
  auto_install_deps parameter.
- config.py: SUPPORTED_CLUSTER_TYPES is now ("local", "ssh", "slurm",
  "huggingface").
- decorator.py: the provider/instance_type/region parameters and the
  seven Kubernetes auto-provisioning ones, the k8s readiness branches in
  both the sync and async paths, and the k8s auto-provisioning check in
  _choose_execution_mode. The kwargs passthrough list keeps only the
  names a surviving backend reads.
- utils.py: _create_pbs_script and _create_sge_script and their dispatch;
  normalize_memory's kubernetes/pbs/sge targets.
- cli_credentials.py: the AWS, Azure, GCP, Kubernetes and Lambda Cloud
  collectors and validators, and their entries in the setup wizard and
  the credential test loop (883 -> 497 lines).

Two things deliberately kept:

hf_hardware, hf_username and hf_sdk look like HuggingFace *Spaces* fields
and sit under a comment that said so, but hf_jobs.py reads them as
fallbacks for hf_flavor and hf_namespace. They stay; the comment is
corrected.

New in config.py: REMOVED_CLUSTER_TYPES and _REMOVED_SETTINGS, checked in
load_config before the difflib path. Without them an existing clustrix.yml
carrying k8s_namespace gets "did you mean ...?" pointed at an unrelated
field, and cluster_type: pbs gets no explanation at all. Now each names
the backend and its tracking issue (#140-#146).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Docs: purge removed backends from the tutorial notebooks

Rewrites the PBS/SGE/Kubernetes/cloud config examples in complete_api_demo and
cluster_config_example, drops the cost-monitoring example from basic_usage,
fixes the dead links to the deleted pbs/kubernetes/cost_monitoring notebooks,
and deletes the now-unreferenced widget_gcp.png / widget_lambda.png
screenshots.

* Issue #147: make the real-world runner able to fail

main() dropped every runner.run_*_tests() return value and never called
sys.exit, so the script exited 0 no matter what. The pre-push hook guards
each category with `if ! python scripts/run_real_world_tests.py --<cat>`,
which therefore could never fire: four categories printed "failed" and the
hook still announced "All real-world tests passed!" and allowed the push.

Same class as the flake8 --exit-zero and mypy continue-on-error steps
fixed in #138 -- a check that reports problems but cannot fail.

Also fixes the second half of #147: the failure message printed only
result.stdout, which was empty in all four observed failures because a
pytest collection error goes to stderr. _report_failure now prints the
exit code, stdout, stderr, and says so explicitly when there was no
output at all.

Verified by appending a deliberately failing test to
tests/real_world/test_filesystem_real.py and running the script:

    EXIT CODE: 1
    ❌ Filesystem tests failed (exit 1)
        assert False, "deliberate failure to verify exit-code propagation"
    E   AssertionError: deliberate failure to verify exit-code propagation

and, with that test removed, the same command exits 0. Before this change
the failing case also exited 0 with an empty message body.

Also drops the removed backends from two scripts: the kubernetes tutorial
entry in check_docs_examples' _SECTION_BOUNDS (that page is deleted) and
the aws/azure/gcp/lambda_cloud entries in the credential display names.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Remove tests for deleted unverified backends (kubernetes, pbs, sge, cloud)

Deleted test modules that exist solely to exercise backends removed from
the package: Kubernetes, PBS, SGE, Lambda Cloud, direct cloud compute and
container-registry validators, plus the reference kubernetes workflow.
tests/integration lost its eight Kubernetes auto-provisioning scripts.

* Have the enhanced widget read SUPPORTED_CLUSTER_TYPES

Its dropdown carried a third hardcoded copy of the backend list, offering
pbs, sge, kubernetes, aws, azure and gcp -- none of which the executor can
dispatch any more. Reading the constant is what stops the copies drifting;
that is why the constant exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Docs: record backend + cost-monitoring removal in README, CLAUDE.md, CHANGELOG

README and CLAUDE.md now carry a single 'not currently supported' note next to
the supported-backends table, naming tracking issues #140-#146 and saying the
backends are planned for a future update without promising a date. The Cloud
Providers and Cost Monitoring sections are replaced by that note.

CHANGELOG records both removals under 0.2.0 as BREAKING, including the five
cost-monitoring functions that no longer exist. The 'Implemented but
unverified' section is gone: those backends no longer exist in the code.

docs/aws/ is kept -- scripts/aws/ cleanup tooling still needs those IAM
permissions -- with a banner marking each guide historical.

* Strip PBS/SGE/Kubernetes branches from mixed real-world test helpers

cluster_job_validator: ClusterType now only SLURM and SSH; the qstat and
kubectl branches are gone.
test_cluster_job_system: the required-test-file list no longer asserts the
existence of the deleted pbs/sge/kubernetes submission tests.
run_cluster_job_tests: availability probe and --cluster choices reduced to
slurm/ssh.
test_advanced_schedulers_comprehensive: PBS and SGE submission tests removed,
remaining tests are SLURM-only.

* Clean up formatting left behind by the backend deletions

Removing the PBS/SGE/Kubernetes/cloud blocks left stranded blank runs
(E303, W391) in the three executor modules and an unused `import sys` in
cli_credentials. black and flake8 are clean on these four files now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Remove Kubernetes/Kind from local test infrastructure

docker-compose no longer starts a kind control plane; setup_test_infrastructure
drops the kind/kubectl dependency checks, the Kind cluster config and RBAC
bootstrap, the kubernetes block in test_infrastructure.json, the KUBECONFIG
export and the Kind teardown.

* Tests: drop removed-backend cases from config/cli/utils/scheduler tests

Deletes tests/unit/test_backends_placeholder_hosts.py (Azure/GCP/Lambda
providers are gone), the PBS and SGE job-script tests in test_utils.py, and
the PBS/SGE parametrisations in test_backends_schedulers.py. Adds coverage
for the new load_config errors on removed cluster_types and settings, and
makes test_all_cluster_types read SUPPORTED_CLUSTER_TYPES instead of a stale
hardcoded list.

* Docs: drop removed-backend docs from the internal testing guides

Deletes docs/kubernetes_testing.md and the three PRICING_* guides outright --
they document the Kubernetes backend and the cost/pricing API, all of which
are gone from the code.

Rewrites the dead cluster_type="kubernetes" examples in testing_guidelines.md
and migration_to_real_tests.md against HuggingFace Jobs and SLURM, removes the
PBS/SGE/Kubernetes sections and the deleted test-file references from
REAL_CLUSTER_JOB_TESTING.md, and adds a scoping note to CREDENTIAL_SETUP.md
saying which credentials still reach an execution backend.

* Remove credentials for unverified backends (aws/azure/gcp/kubernetes/lambda_cloud)

Credential plumbing for backends that were never verified against real
hardware is deleted along with the backends themselves. Retained: ssh
(used by the ssh and slurm backends), huggingface (HF Jobs) and local.

- credential_manager.py: drop the provider entries from all three
  credential sources, from every provider list, from the generated .env
  template, and delete ensure_kubernetes_provider_credentials plus its
  module-level wrapper (no remaining callers anywhere in the tree).
- secure_credentials.py: drop ValidationCredentials.get_aws_credentials,
  get_gcp_credentials, get_lambda_cloud_credentials and the unconditional
  get_docker_credentials stub.
- setup_validation_credentials.py: drop the AWS/GCP/Lambda/Docker entries
  and the pointers to two validation scripts that do not exist.
- test_credential_manager.py: retarget the AWS/Azure assertions at ssh and
  huggingface, and add parametrized tests asserting the removed providers
  stay unresolvable even with their env vars set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Docs: final sweep -- CHANGELOG consistency, Lambda Cloud creds, stale cluster_type comment

The 0.2.0 'Fixed' list described defects in backends the same release removes;
the Kubernetes and cloud-placeholder entries are dropped, the PBS one is
generalised, and a short preface says why the remainder are kept.

Removes the Lambda Cloud credential setup from CREDENTIAL_SETUP.md and fixes
the 'or "pbs", "sge"' comment in the SSH key automation notebook.

* Remove the dead hf_sdk setting and the unreachable credential script

hf_sdk was a HuggingFace *Spaces* concept -- the gradio/streamlit/static
SDK a Space runs under. hf_jobs.py never reads it; the names it does read
are hf_token, hf_namespace, hf_username, hf_image, hf_flavor, hf_hardware,
hf_allow_gpu_flavors, hf_payload_repo and hf_job_timeout. With Spaces gone
the field configured nothing, so it is removed from ClusterConfig and from
the legacy widget, and added to the removed-settings table so an existing
config file gets an explanation rather than a difflib guess.

scripts/setup_validation_credentials.py is deleted. Every run of it ended
at the same place:

    ❌ 1Password CLI not available!
    📥 To install 1Password CLI: macOS: brew install --cask 1password-cli

main() returns 1 there and never reaches the setup guide or the credential
test. SecureCredentialManager.is_op_available() has returned False
unconditionally since 1Password support was removed in #97, so the script
has been unreachable past its first check and was telling readers to
install a CLI clustrix no longer uses. `clustrix credentials setup` and
`clustrix credentials test` already do the job for real. scripts/README.md
now points at those.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Drop hf_sdk from the widget tests and the sample config

Follows the field's removal: it configured the HuggingFace Spaces SDK and
nothing reads it now. test_widget_fixes.py: 8 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Docs: follow the code -- Kind/Kubernetes test infra and cloud credentials are gone

tests/infrastructure/docker-compose.yml no longer starts a Kind cluster and
clustrix.credential_manager now reads only SSH_* and HF_*. Updates the README's
test-infrastructure list, the migration guide's infrastructure validator, and
the CREDENTIAL_SETUP scope note to match.

* Docs: mark issue_71_implementation_summary.md as a historical record

It describes a Kind/Kubernetes test service that no longer exists.

* Drop cloud-provider credential plumbing from real-world tests

RealWorldCredentialManager loses get_aws/azure/gcp/lambda_cloud/kubernetes
credentials and the matching env-var exports; TestCredentials, the
aws/azure/gcp conftest fixtures and the aws_required/azure_required/
gcp_required markers go with them, since nothing retained consumes them.

test_credential_access.py::test_validation_credentials was calling
ValidationCredentials.get_lambda_cloud_credentials/get_aws_credentials/
get_gcp_credentials, none of which exist on that class -- the test raised
AttributeError. It now probes the two credential types the class actually
exposes (HuggingFace, SSH).

* Tests: restate executor/config tests over the backends that remain

Removes the PBS, SGE, Kubernetes and cloud-provider cases from
test_executor.py, test_enhanced_features.py, test_config_real.py,
test_integration.py and test_config_file_permissions.py. Where the property
under test was general (save/load round trip, a failed cancellation keeping
the job tracked, an SSH script carrying no scheduler directives) it is
restated over slurm/ssh/huggingface rather than deleted.

* Update real-world runner, visual tests and progress doc for retained backends

run_real_world_tests: drop the cloud_providers and kubernetes categories,
the kubectl/AWS/GCP/Azure availability probes and their CLI choices.

test_visual_verification: the widget profile assertion still required an
"AWS Batch" profile after the profile itself became HuggingFace Jobs, so it
could never pass; assertion updated to match. The synthetic matplotlib demo
labels no longer advertise PBS/SGE/K8s/cloud providers as supported.

REFACTORING_PROGRESS.md: rows describing deleted modules corrected and a
dated backend-removal section added.

* Correct stale references to deleted backends in integration gate and SSH validator

tests/integration/conftest.py named test_eks_permissions.py and
test_aws_eks_debug.py as the motivating examples; neither file exists any
more, so the rationale is restated without them. The guard itself is
unchanged -- it still stops collection directory-wide.

validate_ssh_cluster_access no longer probes remote hosts for PBS, SGE and
LSF binaries, since clustrix cannot submit to any of them.

* Unblock collection: drop the deleted kubernetes reference workflow

tests/reference_workflows/kubernetes_workflows.py went with the backend,
but test_reference_workflows.py still imported two workflows from it, so
`pytest tests/` aborted during collection and no test in the suite ran.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Tests: narrow scheduler-shape tests to slurm/ssh, drop cloud result path

test_two_venv_execution and test_script_injection are reparametrised over the
backends create_job_script still dispatches. The SSH job directory is
defended by shlex quoting rather than by directive validation, so that is now
asserted directly instead of being folded into the directive test, where SSH
would have passed vacuously. The cloud-worker signing tests go with
executor_cloud.py; the equivalent guarantees are covered by
TestGeneratedWorkerSignsWhatItWrites, TestVerificationFailsClosed and
tests/unit/test_hf_jobs.py.

* Apply black formatting across tests/real_world and tests/integration

test_gpu_detection.py and test_ssh_real.py were already unformatted before
this branch; fixed here rather than left for CI to trip over.

* Tests: sweep remaining removed-backend references out of owned files

Drops the Kubernetes fixture/test from test_executor_real.py, the
fractional-CPU Kubernetes edge case, and tests/unit/test_functions.py (a
Kubernetes helper module nothing imported). Repoints the auth-fallback
profiles and the conftest docstring at backends that still exist.

* Tests: @cluster(platform=...) proved nothing once platform became **kwargs

* Tests: black formatting

* Reject a removed backend where the user can act on it

A removed cluster_type was only caught by load_config. Every other route
in -- ClusterConfig(...) directly, configure(...), or a config already in
memory -- carried it all the way to ClusterExecutor.submit_job, which
checked the type *after* self.connect(). So asking for a backend that no
longer exists cost an SSH round trip to a host that was never going to be
used, and then said only "Unsupported cluster type: pbs".

validate_cluster_type() is now the single check, called from
__post_init__, from configure(), from load_config and from the executor
before it connects. It distinguishes three cases rather than two: a
supported type, a *removed* one (named, with why it went and its tracking
issue), and an ordinary typo.

configure() also validates before it applies anything. It previously
setattr'd its way through kwargs and raised partway, so a call that failed
had still changed the live configuration.

Evidence, all against real files on disk:

  load_config, file containing `cluster_type: pbs`
    cluster_type='pbs' is no longer implemented. It was removed in v0.2.0
    because it had never been verified against real hardware. Its return
    is tracked in issue #140. Supported types are: local, ssh, slurm,
    huggingface.

  load_config, file containing `k8s_namespace: compute`
    contains unknown setting(s): k8s_namespace configured Kubernetes,
    which has been removed (see issue #142)

  load_config, file containing a genuine typo `cluster_hostt`
    contains unknown setting(s): cluster_hostt (did you mean cluster_host?)

  ClusterConfig(cluster_type="kubernetes") -> named, issue #142
  configure(cluster_type="aws")            -> named, issue #143
  configure(k8s_namespace="compute")       -> named, issue #142

  cluster_type after a rejected configure(): 'slurm' (unchanged)

The did-you-mean path is deliberately still reachable: a removed setting
must not look like a spelling mistake, and a spelling mistake must not
look like a removed setting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Tests: pin the CLI cluster-type choices to SUPPORTED_CLUSTER_TYPES

Asserts the CLI offers every supported backend and refuses every removed
one, so the two lists cannot drift apart again.

* Tests: comment named a removed backend

* Tests: host-key tests were writing into the developer's real known_hosts

paramiko's AutoAddPolicy saves accepted keys back to whatever file
load_host_keys() named, so test_auto_add_policy_gets_past_host_key_check
appended an [127.0.0.1]:<ephemeral port> line to ~/.ssh/known_hosts on every
run (83 had accumulated locally). When an ephemeral port was reused against a
freshly generated server key, the reject test hit BadHostKeyException instead
of the missing-host-key policy and failed -- the suite was poisoning its own
precondition. The real_ssh_server fixture now points ~ at a per-test
directory. No assertion changed.

* Point five tests at the messages the code now actually produces

All five asserted on wording that validate_cluster_type replaced. In every
case the property under test is unchanged and the assertion is now
stronger, not weaker:

- test_config.py: "no longer implements" -> "no longer implemented".
- test_enhanced_features.py: matched "Unknown configuration parameter",
  which is exactly the generic phrasing the removed-settings table exists
  to avoid. Now asserts the message names k8s_namespace, Kubernetes,
  "removed" and #142, and that it does NOT contain "did you mean".
- test_executor.py, test_executor_comprehensive.py: "Unsupported cluster
  type" -> "is not a supported cluster type", which also lists the
  supported set.
- test_backends_local.py: test_unknown_cluster_type_still_fails_loudly
  built a ClusterConfig with a bad type *outside* its pytest.raises, so
  the now-earlier rejection escaped the block. Rewritten to assert the
  refusal happens at construction, and joined by a second test covering
  the path that motivated keeping the executor's own check: setattr does
  not re-run __post_init__, so a type assigned after construction (what
  configure() and the widget both do) must still be refused at submit.

Full non-billable suite: 1240 passed, 18 skipped, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Reformat with the pinned black, not whichever one is on PATH

Seven files on this branch were formatted by black 25.11.0. The project
pins black==26.3.1 in both pyproject.toml and setup.py, and that is what
CI installs and checks against -- the two versions disagree, so a local
`black --check` passed while CI would have failed the lint job.

Reformatted with 26.3.1. `black --check clustrix/ tests/ scripts/` is now
clean across all 223 files, and flake8 is clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Delete tests/test_config.yml, a committed test artifact

It is a saved *profiles* file, not a configuration file -- `load_config`
has never been able to read it:

    ValueError: tests/test_config.yml contains unknown setting(s):
    active_profile; profiles

Nothing references it. Every "test_config.yml" in the suite is a name
composed inside a tmpdir. It is exactly the stray that tests/conftest.py's
own docstring describes as leaking out of the suite before the config
directory was redirected, and it was still carrying `aws_*`, `azure_*`,
`k8s_*` and `cost_monitoring` keys for backends that no longer exist.

Suite after removal: 1240 passed, 18 skipped, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* CHANGELOG: describe the errors the code actually raises

The removal section said a removed backend raises "Unsupported cluster type"
at submit time. validate_cluster_type changed both the message and the
timing -- it is now refused at construction, at configure(), at load_config
and in the executor before it connects. Quotes the real messages, including
the typo case that must keep its did-you-mean hint.

Also records the API surface that went with the backends (configure's
auto_install_deps, ten @cluster parameters, the config fields, hf_sdk), and
adds the two test-suite defects found in this pass: #147 and the host-key
tests writing into the developer's real known_hosts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: record completion, the gate results, and what is left

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Drop the packaging extras for backends that no longer exist

`pip install clustrix[aws]` pulled boto3 and the Kubernetes client for code
that is not in the package. The `kubernetes`, `aws`, `azure`, `gcp` and
`cloud` extras are gone, and the cloud SDKs are out of `test` and `all` too.
Nothing in `clustrix/` imports any of them:

    $ grep -rn "import boto3\|from kubernetes\|from azure\|from google.cloud" clustrix/
    (no matches)

`scripts/aws/` still uses boto3, but deliberately imports it lazily with its
own message -- "boto3 is not a clustrix dependency; these AWS utilities are
optional" -- so it never needed an extra either.

CI's install line drops the extra with them:

    - pip install -e ".[dev,test,kubernetes,widget]"
    + pip install -e ".[dev,test,widget]"

Verified by resolving the new line in a clean 3.11 venv: `pip install
--dry-run -e ".[dev,test,widget]"` succeeds and the resolved set contains no
boto3, kubernetes, azure-* or google-* package.

Also drops "kubernetes" from the package keywords, which advertised it on
PyPI, in favour of the backends that exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* docs: make the -W sphinx build pass by resolving IPython's forward ref

sphinx_autodoc_typehints evaluates annotations across a documented class's
whole MRO. ClusterfyMagics subclasses IPython's Magics, which annotates
`shell: InteractiveShell` behind a TYPE_CHECKING guard, so the name does not
exist in IPython.core.magic at runtime and get_type_hints() cannot resolve it.
That emitted a forward_reference warning, fatal under sphinx-build -W and so
blocking the docs gate entirely. Bind the name instead of adding the category
to suppress_warnings, which would also hide the same class of warning in
clustrix's own code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* docs: notebook cross-references were rendering as literal `:doc:`/`:ref:` text

nbsphinx renders a notebook markdown cell as Markdown, not reStructuredText,
so an rst role in one is never resolved. The built HTML showed, verbatim,
`:doc:<code>../ssh_setup</code>` and `:ref:<code>execution-model</code>` --
eight dead cross-references across five notebooks, and meaningless text for
anyone reading the same notebook in Colab or on GitHub, which is where the
Open in Colab badges send them.

Replaced with absolute readthedocs links, which resolve in all three contexts.
Each target URL was fetched and returned 200.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Stop handing cloud credentials to jobs that cannot use them

real-world-tests.yml exported LAMBDA_CLOUD_API_KEY, GCP_PROJECT_ID,
GCP_JSON, AWS_ACCESS_KEY_ID and AWS_ACCESS_KEY into four steps. Every
backend that read them is deleted, so they were doing nothing except
widening the blast radius of a compromised step: a job that cannot use a
credential should not be given one.

What remains maps exactly to the backends that survive -- CLUSTRIX_USERNAME
and CLUSTRIX_PASSWORD for ssh/slurm, HF_USERNAME and HF_TOKEN for
HuggingFace Jobs -- and the check-secrets gate already keys on precisely
those.

Worth recording alongside this: every step in this workflow invokes
`python scripts/run_real_world_tests.py --<category>` as a bare command,
and Actions fails a step on a non-zero exit. Because that script exited 0
no matter what (#147, fixed in 9b90eaa), **this workflow could not fail
either** -- the same defect as the pre-push hook, in the one workflow whose
entire purpose is to validate against real clusters. It can fail now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: record the follow-on cleanups and the black version trap

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* docs: fix stale notebook claims, add missing badges and failure guidance

Verification found four classes of defect, all fixed in the notebooks:

Missing Colab badges. cluster_config_example.ipynb and filesystem_tutorial.ipynb
had none, while the other five did. Added, pointing at the same path the file
occupies in the repo.

Widget-on-import is no longer true. ssh_tutorial.ipynb and
ssh_key_automation_tutorial.ipynb both told the reader to look for a widget
that 'appeared automatically' when clustrix was imported. Importing only
registers the %%remote magic. ssh_key_automation_tutorial went further and
walked through an 'SSH Key Setup' section with its own host/user/password
fields and a 'Setup SSH Keys' button -- that is the legacy
EnhancedClusterConfigWidget, not the ModernClustrixWidget %%remote shows,
which has an 'Auto setup SSH keys' button in its Connection section.

Wrong Colab secret names. The tutorial told readers to store a Colab secret as
CLUSTER_PASSWORD_CLUSTER_UNIVERSITY_EDU. get_cluster_password()'s Colab branch
tries CLUSTER_PASSWORD_<raw hostname> first -- dots intact, not upper-cased --
so that name is never read from Colab secrets. Replaced with the actual list,
in the actual order, for both the Colab and the environment-variable paths.

Removed features in Next Steps. ssh_key_automation_tutorial still pointed at
cloud provider integrations and cost monitoring, both deleted in v0.2.0.

Also added what was missing rather than wrong: a job-directory autopsy for
slurm_tutorial and ssh_tutorial (exact path, every file in it, and the three
distinct failure modes), a 'When You Would Not Want This' section for
basic_usage, and a step-by-step account of what setup_ssh_keys() really does.
Every claim was checked against clustrix/ before it was written down.

All seven notebooks re-executed; the only remaining failures are the
placeholder hostnames in slurm_tutorial and ssh_tutorial, which need a real
cluster. sphinx -b html -W --keep-going passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Issue #123: bound the job wait loop

_wait_for_scheduler_result polled under a bare `while True` with no
deadline and no timeout field anywhere in ClusterConfig. A job that never
reached a terminal state -- held by the scheduler, sitting behind a queue
that never cleared, a node stuck draining -- hung the caller forever, with
no diagnostic and no way out but Ctrl-C. This is on the primary path of
both verified scheduler backends.

New `job_wait_timeout`, default 86400 (24 hours). Deliberately generous:
a real HPC queue wait legitimately runs into hours, so a short default
would break correct usage. Set it to None for the old unbounded wait.

On expiry the job is deliberately NOT cancelled -- it may still be queued,
and killing someone's allocation because the client got bored is not this
function's call. The error names the job, the elapsed limit and the
setting that controls it, the last status seen, and the remote directory
the result can still be collected from:

    TimeoutError: Job job_1 did not finish within 2s
    (config.job_wait_timeout). Its last known status was 'running'. The job
    has NOT been cancelled; its files are at
    /scratch/someone/.clustrix/jobs/job_1 on the cluster. Raise
    job_wait_timeout, or set it to None to wait indefinitely.

tests/unit/test_job_wait_timeout.py covers it with no mocks: a real
ClusterExecutor running the production loop, against a real subclass of
the real SchedulerManager whose job never leaves the queue. Four tests --
it gives up near the deadline rather than a multiple of it, the default is
finite, None really does remove the deadline (observed still polling well
past a deadline that would have fired), and an unknown job id is rejected
before any polling happens. 4 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Issue #124: run the documentation example checker in CI

scripts/check_docs_examples.py executes every code block the documentation
publishes -- 143 of them, 110 for real -- and it passes. Nothing ran it:

    $ grep -rn check_docs_examples .github/
    (no matches)

So the docs were correct only for as long as someone remembered to check by
hand, and stale examples have been this project's largest recurring defect
class. The step runs once on ubuntu/3.11; the examples are not
version-specific, so running it seven times would only cost minutes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Issue #115: give coverage a floor that can actually fail

`grep -rn fail_under` across pyproject/setup.cfg/pytest.ini/CI returned
zero hits. The old `fail_under = 90` was removed because it was set
against a coverage number nobody could reproduce -- correct -- but nothing
replaced it, so the project measured 68% and gated on nothing.

Measured on this tree, with the command CI runs:

    TOTAL    7180 stmts    2311 missing    68%
    1244 passed, 18 skipped, 17 deselected

`fail_under = 66` -- two points of headroom so a version-to-version
difference across the 3.10/3.11/3.12 matrix cannot turn a green run red,
while a real regression still does. A floor and a ratchet, not a target;
90 remains the goal in #98 and this is not a claim to have reached it.

Verified it fires rather than being decorative, by running a single test
file under the same coverage command:

    FAIL Required test coverage of 66.0% not reached. Total coverage: 14.40%
    EXIT CODE: 1

CI already passes `--cov=clustrix` on every matrix job (tests.yml:81), so
this needed no workflow change to take effect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Fix the docs-example CI step: it ran after a non-editable install

The step I added in b67a56d failed on ubuntu/3.11 with:

    File "scripts/check_docs_examples.py", line 695, in main
      rel = target.path.relative_to(REPO_ROOT)
    ValueError: '/opt/hostedtoolcache/.../site-packages/clustrix/config.py'
    is not in the subpath of '/home/runner/work/clustrix/clustrix'

It sat after "Test installation", which does a non-editable `pip install .`.
After that, `import clustrix` resolves to site-packages, so
`inspect.getsourcefile` returned a path outside the checkout and the
relative_to blew up.

Two changes, because the ordering bug hid a real one:

1. The step now runs BEFORE "Test installation". That is where it belongs
   -- it checks the docs in this checkout against the code in this
   checkout.

2. The checker now refuses a foreign install by name instead of crashing
   inside pathlib. Getting the path arithmetic to survive would have been
   worse than the crash: the examples would have been silently checked
   against a *different copy* of the code, and passed while proving
   nothing.

Reproduced the failure locally in a venv with a non-editable install, run
from outside the repo, and confirmed the new message:

    documented module 'clustrix.config' imports from
    .../site-packages/clustrix/config.py, which is outside this checkout
    (/Users/jmanning/clustrix). The examples here would be checked against
    a different copy of the code. Install the package editable
    (pip install -e .) or run this before a non-editable install.

Normal path unaffected: 143 blocks checked, 143 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Notes: PR #149 green, 15 issues closed, and the backlog's bad numbers

Records which issues were closed with what evidence, the re-measured
figures that contradict the backlog, and the two items that need the user.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

* Add a Tests Status gate so branch protection has a stable name to require

`master` had no branch protection at all -- `gh api
repos/ContextLab/clustrix/branches/master/protection` returned "Branch not
protected" -- so every green run this project has fixed was advisory. A red
run could always be merged past.

Protection needs required check names. The Tests workflow had none that is
stable: pinning it to the seven `test (os, version)` jobs individually
means protection silently stops covering any combination added later, and
breaks whenever the matrix changes. Fast CI already solved this with its
`CI Status` aggregator; this is the same pattern for the workflow that runs
the actual test suite.

`if: always()` matters: without it the job would be *skipped* when a
dependency fails, and a skipped required check does not block a merge --
the gate would be worse than none.

The condition names every job in `needs` explicitly, because the version of
this in fast_ci.yml had security-scan in `needs` but not in its loop, and
so reported "All CI checks passed" while the security scan burned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pre_push_check.py can never pass: 92 flake8 findings, incl. a real f-string bug in test_direct_gpu_detection.py

1 participant

@jeremymanning