Skip to content

style+fix: conform to the fleet's black and ruff gates (B and C) - #58

Closed
stranske wants to merge 2 commits into
mainfrom
claude/ci-conform-format-lint
Closed

style+fix: conform to the fleet's black and ruff gates (B and C)#58
stranske wants to merge 2 commits into
mainfrom
claude/ci-conform-format-lint

Conversation

@stranske

Copy link
Copy Markdown
Owner

Conforms this repo to the fleet's format and lint gates. B and C onlypyproject.toml (A) is held pending the src/ layout question.

Two commits, deliberately separate:

commit what review effort
4117aa2 black --line-length 100 — 126 files, purely mechanical skip it; it's the formatter's output
14ccb6d 61 ruff findings, incl. 3 real defects this is the one to read

Why these were failing

pr-00-gate.yml calls stranske/Workflows/.github/workflows/reusable-10-ci-python.yml@main. Its two relevant jobs run, verbatim:

black --check --line-length 100 --exclude '(\.venv|\.workflows-lib|node_modules)' .   # line 690
ruff check --select E4,E7,E9,F --output-format github --extend-exclude .workflows-lib .   # line 1155

The ruff line is the no-explicit-selection fallback — pyflakes plus serious pycodestyle, i.e. real defects rather than style. Both now pass.

A measurement correction worth recording

My first pass reported 914 ruff errors and 180 files needing black, and concluded the gate was unreasonable. Both numbers were artefacts of running the tools bare: with no pyproject.toml present they fall back to their own defaults (line length 88; the full ruff rule set), not to what CI runs. At CI's real settings it was 126 files and 79 findings — and after black split the 18 E702 semicolons, 61.

Take the command line from reusable-10-ci-python.yml, never from the tool default.

What black actually changes, measured

Code only: tuple/list literals exploded one per line, implicit string concatenation re-indented, a; b split. It also collapses the column-aligned padding before trailing # comments to two spaces, which this tree used extensively — 244 comment-bearing lines are byte-identical after whitespace normalisation. No comment or docstring prose is reworded or reflowed anywhere. (I initially claimed "0 comment lines touched" from a single-file sample; the inline-alignment collapse is real, and cosmetic.)

black verifies AST equivalence on every file it rewrites.

The three real defects ruff found

  1. test_observability_activation.py set "route_weights" twice in one dict literal (lines 52 and 90). Same value, so no behaviour changed — but one silently overrode the other.
  2. verify.py unpacked fc, fp in verify() while only fc is used — a leftover from the refactor that moved the passed-floor comparison into _floor_problems. Removed there, kept in _floor_problems, which genuinely uses both.
  3. capability_admission.py computed cited = {d["record"] for d in com["dangling_citations"]} and asserted nothing on it. This repo's founding defect in miniature: a value computed for a check that no longer checks. The dead binding is removed; the missing assertion is not invented here — that needs a decision about what it should assert.

Deliberately not repaired

keepalive_evidence.py built haystack = f"#{issue_number} {issue_title}" and then searched issue_title alone — so a bare #N reference in a title is never matched. Wiring haystack in would change what the keepalive evidence path matches, and that is Brain input (CLAUDE.md §2). A behaviour change has no business riding along in a lint pass. Only the unused binding is removed, which changes nothing.

Also load-bearing and preserved: in exp_abcd.py the lambda→def conversion keeps the default-arg capture repo=meta["repo"], exp_id=edir.name verbatim in the signature — it binds this iteration's values so a later loop turn cannot rebind them.

Method note

My first edit replaced the wrong fc, fp = ... occurrence and broke _floor_problems. ruff --select F821 named it immediately — "Undefined name fp", three sites. That is the argument for this gate existing rather than being configured away.

Test gate

python3 verify.py368 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates. Floor and ceilings untouched: nothing was added, removed or renamed.

Both gates re-run exactly as CI invokes them: black --check192 files unchanged; ruff check --select E4,E7,E9,FAll checks passed.

Still red after this, and why

python 3.12 / 3.13 / typecheck-mypy need pyproject.toml (CI passes --cov-config=pyproject.toml, which currently raises ConfigError). That is commit A, held pending the src/ layout decision.

🤖 Generated with Claude Code

Tim Stranske and others added 2 commits August 23, 2026 07:54
…no logic change)

`pr-00-gate.yml` calls `stranske/Workflows/.github/workflows/reusable-10-ci-python.yml@main`,
whose format job runs exactly:

    black --check --line-length 100 --exclude '(\.venv|\.workflows-lib|node_modules)' .

That gate started firing on this repo and failed, because the tree had never been black-formatted.
This runs the formatter with those exact arguments, so the check and the tree agree by construction
rather than by coincidence.

WHOLLY MECHANICAL. `black` verifies AST equivalence on every file it rewrites, and `verify.py` is
green on the result: 368 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates.
No floor or ceiling moves — nothing was added, removed or renamed. Review the next commit instead;
this one is the formatter's output.

WHAT IT ACTUALLY CHANGES, measured rather than assumed. Code only: tuple/list literals exploded one
per line, implicitly-concatenated strings re-indented, `a; b` split onto two lines. It also collapses
the column-aligned padding before a trailing `#` comment to two spaces, which this tree used
extensively — 244 comment-bearing lines are byte-identical after whitespace normalisation. No comment
or docstring PROSE is reworded or reflowed anywhere; black does not touch either.

A note on measurement, because the first attempt at this was wrong. Running bare `black --check .`
reports 180 files, and bare `ruff check .` reports 914 errors — both artefacts of this repo having
no `pyproject.toml`, so the tools fall back to their own defaults (line length 88; the full ruff
rule set) rather than to what CI runs. At CI's real settings the numbers are 126 files and 79 lint
findings. Always take the command line from `reusable-10-ci-python.yml`, not from the tool default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`reusable-10-ci-python.yml:1155` runs `ruff check --select E4,E7,E9,F` when a repo declares no
explicit rule selection — pyflakes plus the serious pycodestyle errors, i.e. real defects rather
than style. This clears all of them. `ruff check --select E4,E7,E9,F --extend-exclude
.workflows-lib .` now passes.

The previous commit (black) already removed all 18 `E702` semicolon statements, leaving 61.

MECHANICAL (54):
  * F401 x25 unused imports, F541 x2 empty f-strings — `ruff --fix`. Each of the five that could
    plausibly have been a re-export (`ux_review.MIN_EVALUATORS`, `EVALUATOR_TOPUP_ORDER`,
    `periodic_report.execution_profiles`, `capability_targets.repo_knowledge`,
    `capability_propensity.env_prereq`) was grepped for use elsewhere first; none had any.
  * E741 x19 ambiguous `l` — every instance is a comprehension variable whose scope is its own
    line, renamed to what it holds (`lb` for labels, `link`, `lab`, `ln` for a JSON line).
  * E731 x7 lambda assignments -> `def`. In `exp_abcd.py` the default-arg capture
    (`repo=meta["repo"], exp_id=edir.name`) is LOAD-BEARING — it binds this iteration's values so a
    later loop turn cannot rebind them — and is preserved verbatim in the def signature.
  * E402 x1: a stray mid-file `import math` in `feedback.py`, moved to the import block. Not a
    cycle-avoidance lazy import; those live inside functions and stay there.

REAL DEFECTS the gate found (3):
  * `test_observability_activation.py` set `"route_weights"` TWICE in one dict literal (lines 52 and
    90). Same value, so no behaviour changed, but one silently overrode the other.
  * `verify.py` unpacked `fc, fp` in `verify()` while only `fc` is used — a leftover from the
    refactor that moved the passed-floor comparison into `_floor_problems`. Removed there and kept
    in `_floor_problems`, which genuinely uses both.
  * `capability_admission.py` computed `cited = {d["record"] for d in com["dangling_citations"]}`
    and asserted nothing on it. That is this repo's founding defect in miniature — a value computed
    for a check that no longer checks. The dead binding is removed; the MISSING assertion is not
    invented here.

DELIBERATELY NOT REPAIRED, flagged instead: `keepalive_evidence.py` built
`haystack = f"#{issue_number} {issue_title}"` and then searched `issue_title` alone, so a bare `#N`
reference in a title is never matched. Wiring `haystack` in would change what the keepalive
evidence path MATCHES, and that is Brain input (CLAUDE.md 2) — a behaviour change has no business
riding along in a lint pass. Only the unused binding is removed, which changes nothing.

A note on method: the first edit here replaced the wrong `fc, fp = ...` occurrence and broke
`_floor_problems`. `ruff --select F821` named it immediately ("Undefined name `fp`", three sites) —
which is the argument for this gate existing rather than being configured away.

Verified with `python3 verify.py`: 368 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43
can-fire, 5/5 gates. Floor and ceilings untouched — nothing added, removed or renamed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 5 minutes

Limit details: You’ve used the included review currently available. Your 74 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b475fb8f-810c-40a9-bd53-02dd95095122

📥 Commits

Reviewing files that changed from the base of the PR and between af6654d and 14ccb6d.

📒 Files selected for processing (126)
  • adapters.py
  • adversarial.py
  • agent_auth_check.py
  • backlog.py
  • cadence_registry.py
  • capabilities.py
  • capability_activation_audit.py
  • capability_admission.py
  • capability_advisor.py
  • capability_compiler.py
  • capability_effectiveness.py
  • capability_firing_monitor.py
  • capability_ir.py
  • capability_lifecycle.py
  • capability_matcher_proposals.py
  • capability_opportunity.py
  • capability_outcome_bridge.py
  • capability_propensity.py
  • capability_recurrence_check.py
  • capability_targets.py
  • capacity.py
  • ccusage_reconcile.py
  • claims.py
  • codemod_lane.py
  • completion_event_adapter.py
  • consumer_sync_artifact_ingest.py
  • consumer_sync_shadow.py
  • cross_repo_lane.py
  • dispatcher.py
  • dry_seam_audit.py
  • durability_sweep.py
  • env_prereq.py
  • epic_lane.py
  • evidence_acquisition.py
  • evidence_schema.py
  • execution_profiles.py
  • exp_abcd.py
  • experiment_recovery.py
  • exploration_backfill.py
  • exploration_collection.py
  • exploration_evidence_plan.py
  • exploration_review.py
  • feature_scan.py
  • features.py
  • feedback.py
  • frontend_verify.py
  • gh_capacity.py
  • human_calibration.py
  • issue_quality.py
  • issue_readiness.py
  • judge_reliability.py
  • keepalive_evidence.py
  • keepalive_outcomes.py
  • keepalive_shadow.py
  • keepalive_supervisor.py
  • langsmith_direct.py
  • langsmith_fetch.py
  • langsmith_pull.py
  • ledger_reconcile.py
  • local_verify.py
  • mcp_server.py
  • merge_guard.py
  • model_profile_trial.py
  • model_profile_trial_bridge.py
  • objective_anchor.py
  • observability_dashboard.py
  • outcomes.py
  • partitioned_review.py
  • pattern_miner.py
  • periodic_report.py
  • provision.py
  • range_lane_rollout.py
  • redirect_apply.py
  • redirect_plan.py
  • redirect_policy.py
  • redirect_shadow.py
  • redirect_sweep.py
  • relearn_report.py
  • repo_knowledge.py
  • research_scheduler.py
  • research_subjects.py
  • roles.py
  • router.py
  • runner_effect_bridge.py
  • runtime_ac.py
  • runtime_ac_flow_monitor.py
  • runtime_ac_gate.py
  • runtime_ac_panel.py
  • strategy_experiment.py
  • switch_review.py
  • synthesis_promotion.py
  • test_capabilities.py
  • test_capability_admission.py
  • test_capability_causal_core.py
  • test_capability_epic.py
  • test_capability_lifecycle_e2e.py
  • test_capability_set_coverage.py
  • test_capacity_profiles.py
  • test_completion_events.py
  • test_consumer_sync_artifact_ingest.py
  • test_consumer_sync_shadow.py
  • test_evidence_contract_compiler.py
  • test_experiment_arm_identity.py
  • test_feedback_model_provenance.py
  • test_model_profile_trial.py
  • test_model_profile_trial_bridge.py
  • test_model_tier_resolution.py
  • test_observability_activation.py
  • test_partitioned_review.py
  • test_pattern_miner.py
  • test_playbook_compiler.py
  • test_research_control.py
  • test_role_compiler.py
  • test_roles_lineage.py
  • test_runner_effect_bridge.py
  • test_runtime_ac_flow_monitor.py
  • test_skill_compiler.py
  • test_synthesis_promotion.py
  • test_ux_review.py
  • test_workflow_compiler.py
  • testgen_gate.py
  • testgen_lane.py
  • tick.py
  • ux_review.py
  • verify.py
  • watch.py

Comment @coderabbitai help to get the list of available commands.

@stranske

Copy link
Copy Markdown
Owner Author

Closing — superseded by #60, which has the root cause this PR does not.

I built this from a local measurement without checking open PRs or the improvement log first. #60 got there properly, and its diagnosis invalidates the premise of this PR: all five upstream jobs were dying at a shared install step —

Error: .github/workflows/autofix-versions.env is required; refusing to install unpinned tooling.

Ruff, Black and mypy had never executed on this repo, not once. Every statement about its lint debt, mine included, was inferred. #60 adds the missing pin file, plus ruff.toml + mypy.ini, docs/CI_LINT_BASELINE.md, and toggles that state blocking/drainable/drains-by with a test enforcing it.

It also explains why my 126-file black pass was largely wasted: the repo self-heals formatting via its own autofix workflow (chore(autofix): formatting/lint, adfa5f4 / 9239e06). What autofix leaves behind is exactly the non-auto-fixable residue — the E741/E731/F841/E402/F601 judgement calls — and #60 drains those too, under a ruff.toml selecting ["E4","E7","E9","F","I"] that collapses the Gate/Autofix two-window disagreement into one set. That last part is the real fix and this PR does not have it.

One thing from here worth carrying over if it is not already in #60, since these were verified against live code:

  • test_observability_activation.py sets "route_weights" twice in one dict literal (lines 52 and 90) — same value, so nothing behaves differently, but one silently overrides the other.
  • capability_admission.py computes cited = {d["record"] for d in com["dangling_citations"]} and asserts nothing on it — a value computed for a check that no longer checks. Filed as a chip; the missing assertion needs a decision, not a deletion.
  • keepalive_evidence.py builds haystack = f"#{issue_number} {issue_title}" then searches issue_title alone, so a bare #N in a title is never matched. Deliberately not repaired — it changes what the keepalive evidence path matches, which is Brain input (CLAUDE.md §2). Filed as a chip.

And a correction to my own earlier claim on #58: the --cov-config=pyproject.toml failure is real, but adding pyproject.toml is not the fix. reusable-10-ci-python.yml appends -e '.[app,dev]' at eight sites gated only on [ -f pyproject.toml ], so the file's mere existence makes all five jobs attempt an editable install — and 126 flat root modules have no package and no app/dev extras. It would move the failure earlier, to the install step. #60's ruff.toml + mypy.ini is the correct shape for a non-package repo.

🤖 Addressed by Claude Code

@stranske stranske closed this Aug 23, 2026
stranske pushed a commit that referenced this pull request Aug 23, 2026
…y it stays dead

`_dedupe_candidate` built `haystack = f"#{issue_number} {issue_title}"` and then searched
`issue_title` and `candidate["seed"]` only. ruff F841 flagged the unused local; PR #58 removed
the binding as lint. This decides the question that removal left open — was the combined form
the intent? No, and it is inert rather than merely unused:

  * Substituting `haystack` for `issue_title` in the first `_contains_pr_ref` adds EXACTLY one
    condition, `issue_number == pr_number`. It does not widen the text searched. Checked by brute
    force over 200k random (issue_number, pr_number, title) triples: 0 mismatches against
    `ref(title, pr) or issue_number == pr`.
  * That condition cannot occur: GitHub draws issue and PR numbers from one per-repo sequence.
    Over recorded history — 202 dedupe-eligible candidates (reverted/abandoned keepalive rows
    carrying a PR number, 2026-01-08..2026-08-22, 11 repos) against 4,031 issues in those same
    repos — 0 number collisions.
  * And it is already covered even if it could: every seed `evidence_for_repo` builds opens with
    "PR #{pr_number}", so the adjacent `_contains_pr_ref(candidate["seed"], issue_number)`
    disjunct is already true in exactly that case. Differential run of the real
    `_dedupe_candidate` against a haystack-wired copy over 120k real-shaped candidates, with
    `issue_number == pr_number` forced in ~40% of search hits: 0 behaviour differences.

So the effect on matching is +0 candidates, not merely over history but under any input. The one
way it could ever fire is as a false positive claiming duplication from a numeric coincidence.

Also corrects the premise behind the question: `_contains_pr_ref` ALREADY matches a bare "#N" in
an issue title — the "#" in `#?{n}` is optional and `(?<!\d)` excludes only a preceding digit.

Nothing here changes what the path matches, so CLAUDE.md §2 is satisfied by the change being a
true no-op rather than by deferring it. The genuine widening, if ever wanted, is the sibling
pattern in durability_sweep.py — title -> title+body, which widens the TEXT.

The explanation sits ABOVE the matching loop rather than in the deleted line's slot, so an
identical deletion on both sides merges cleanly with PR #58 instead of conflicting.

Selftest pins the decision with two assertions and a deliberate-break -> revert demonstration:
wiring the haystack in fails assertion 1 (exit 1); removing "PR #<pr>" from the reversal seed
fails assertion 2 (exit 1); revert restores byte-for-byte and both pass (exit 0).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stranske pushed a commit that referenced this pull request Aug 23, 2026
…is redundant

PR #58's entire change to keepalive_evidence.py is two deletions: the dead `haystack` binding
(decided in the previous commit) and this unused `repo = target.split("#", 1)[0]` in the
selftest loop — `record_run` is passed `target`, never `repo`.

Taking both makes this branch's copy of the file a superset of #58's, and since both are now
identical deletions on both sides, the file merges with zero conflicts in either order —
verified with a 3-way merge against origin/claude/ci-conform-format-lint.

`ruff check --select E4,E7,E9,F` is clean for this file and `black --line-length 100 --check`
leaves it unchanged. #58 is separately CONFLICTING against main (it predates #59 and is a
tree-wide black pass), so it needs a rebase regardless — that is not caused by this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@stranske
stranske deleted the claude/ci-conform-format-lint branch August 23, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant