Fix issue #530 CliRunner stderr capture + clean ruff baseline to zero - #551
Merged
Conversation
click==8.1.8 defaults CliRunner(mix_stderr=True), which merges stderr into
result.output and makes result.stderr raise ValueError("stderr not
separately captured"). test_ps_warns_about_unparseable_agentd_rows asserts
on result.stderr directly, so it now builds its own
CliRunner(mix_stderr=False) instead of using the shared `runner` fixture
(which stays mix_stderr=True for every other test in the file, none of
which reads .stderr).
Verified:
uv run pytest packages/doeff-agents/tests/test_cli_agentd.py -v
-> red before fix (ValueError: stderr not separately captured)
-> green after fix, 13 passed
Baseline was 96 errors (29 auto-fixable). Fixed by rule family:
- I001 (16): import-order only, via `ruff check --fix` + manual
consolidation of two mid-file import blocks in
packages/doeff-agents/tests/test_inprocess_report_result.py (isort's
default combine-as-imports=false legitimately splits aliased imports
into one-per-line, matching the existing repo convention in
doeff/__init__.py).
- E402 (15): hoisted to the top-of-file import block where nothing
depended on load order (examples/marker_patterns.py, and both
mid-file blocks in test_inprocess_report_result.py). No sys.path-style
ordering dependency existed anywhere in this baseline, so no per-line
noqa was needed.
- PT018 (11): split every compound `assert a and b[, msg]` into two
separate asserts, preserving short-circuit guard semantics exactly
(e.g. `assert x` before `assert x[0]...` so an empty/None guard still
prevents the IndexError/AttributeError the original `and` prevented)
and preserving the exact failure message on both parts.
- RUF001/RUF002/RUF003 (22): all instances were the same 3 legitimate,
repeated characters — `❯`/`›` (verbatim Claude Code / Codex CLI
prompt-cursor glyphs captured in conformance test fixtures, must stay
byte-identical to real terminal output) and `×` (typographic
multiplication/repeat-count mark in prose comments). Added all three
to `[tool.ruff.lint] allowed-confusables` in pyproject.toml instead of
scattering 22 per-line noqa. This also retired 9 now-redundant
`# noqa: RUF001` comments elsewhere in the repo (RUF100), which were
removed.
- F401 (6): removed genuinely unused imports (verified no re-export
usage via repo-wide grep for each symbol first); kept import-hook
side-effect imports (`import hy`) that autofix wrongly wanted to
delete — see F811 below.
Beyond the top 6 families, the remaining 28 errors were also fixed
mechanically once individually verified behavior-preserving:
- F811 (3): `import hy` is intentionally re-imported inside 3 functions
in packages/doeff-agents/src/doeff_agents/handlers/__init__.py to
activate the Hy import hook lazily at each call site (already had
`# noqa: F401` documenting this). `ruff --fix`'s F811 fix silently
*deleted* these intentional imports (a real behavior change caught
and reverted); fixed instead by adding F811 to the existing noqa.
- PLR0911/PLR0912/PLR0915 (7, across conformance_agent.py x2,
test_s17_endpoint_parity.py, test_s21_resume_fork.py, relaymain.py,
verbs.py): suppressed with
`# noqa: PLR09xx - baseline cleanup keeps existing control flow
unchanged`, following the exact existing convention already used
~40 times across this repo (e.g. doeff/run.py:41,
packages/doeff-core-effects/doeff_core_effects/scheduler.py) for
this same "baseline cleanup, don't refactor control flow" situation.
- F841, C408, F541, PLE2513, RUF100, SIM117: mechanical, verified
behavior-identical (unused-var removal after checking no side
effects; dict()->literal; f-string with no placeholders; literal ESC
byte -> `\x1b` escape, confirmed byte-identical via `od -c`; unused
noqa removal; nested `with` merge).
- SIM102 (production.py): merged nested `if` into one `and` condition,
no side effects between the checks.
- SIM108 (conformance_agent.py): if/else assignment -> ternary, exact
ruff-suggested rewrite.
- SIM105 (conformance_agent.py): try/except Exception/pass ->
`contextlib.suppress(Exception)` (added the import).
- SIM118 (test_s14_identity_persistence_expected_red.py): `.keys()` ->
direct dict iteration, confirmed via the function's own `-> dict`
return type annotation and the `harness.session_row` source that
`row` is a plain dict, not a `sqlite3.Row` (which would iterate over
values, not keys, and thus not be a safe rewrite).
- RUF043 (test_quorum_runtime.py): `match="quorum.*bind"` ->
`match=r"quorum.*bind"`; the `.`/`*` are genuinely intended as regex
wildcards against the real exception message ("quorum parallel joins
must bind..."), so marking it raw (not `re.escape`) is the correct,
string-identical fix.
Held back (3 residual errors — behavior-adjacent, no repo noqa
precedent, left for maintainer judgment rather than guessed at):
- PT017 x2 (test_s10_payload_durability.py:123): rewriting the
try/except+assert to `pytest.raises()` is well-defined but changes
the failure-path diagnostic text (loses the custom "expected
report_result to be rejected..." AssertionError message in favor of
pytest's generic "DID NOT RAISE").
- B904 (relaymain.py:78): adding `from e` changes exception chaining
(`__cause__`) and traceback rendering text, which is a real (if
narrow) behavior difference in error presentation.
Pre-existing failures unrelated to this change, confirmed identical on
both HEAD and this branch via `git stash`:
- packages/doeff-agents/tests/test_sessionhost_substrate_herdr.py (5
tests): live herdr socket protocol mismatch
(`HerdrApiError: missing field 'kind'`), infra/environment issue.
- packages/doeff-conductor/tests/test_k3_workspace_journal_e2e.py (2
tests): `NameError: CreateWorkspace` — TYPE_CHECKING-only import
referenced at runtime in journaled_workspace.py, pre-existing latent
bug untouched by this branch.
- examples/sample_entrypoints.py: `ImportError: cannot import name
'ask' from 'doeff'` at line 12 (untouched by this change) — stale
example, not exercised by any test.
Verification:
uv run ruff check --statistics -> 96 errors baseline
uv run ruff check --fix -> 29 auto-fixed (1 unsafe autofix
reverted: F811 deleting
intentional `import hy`)
uv run ruff check -> 3 errors remaining (PT017 x2,
B904 x1; see "Held back" above)
uv run pytest tests/test_traceback_dedup_frames.py
tests/effects/test_memo_lazy_storage.py
packages/doeff-vm/tests/test_traceback_pure_python_frames.py -> 9 passed
uv run pytest tests/effects/ tests/test_cache_await_spawn_hang.py
packages/doeff-adr/tests/ -> 233 passed, 1 skipped
uv run pytest packages/doeff-agents/tests/ -m "not e2e"
--deselect .../test_sessionhost_substrate_herdr.py -> 1668 passed,
21 skipped
uv run pytest packages/doeff-conductor/tests/ -m "not e2e" -> 418
passed, 2 pre-existing failures (see above)
uv run pytest packages/doeff-agents/conformance/ --collect-only ->
53 tests collected cleanly (heavy E2E harness, not executed)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer decisions on the three items held back by the baseline cleanup: - test_s10_payload_durability.py: rewrite the try/except-assert form to pytest.raises (PT017) — intent preserved via comment; verified green under the canonical hy gate (CONFORMANCE_AGENTD_BIN=doeff-sessionhost) - relaymain.py: raise RuntimeError(...) from e (B904) — explicit cause chaining is strictly better diagnostics, not a behavior risk uv run ruff check: All checks passed (baseline 96 -> 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 25, 2026
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
1 participant
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.
概要
残作業マップ A-3 + A-4(operator「go」承認済み)。lint 基線を green 化し、以後の PR で「差分の赤 = 自分の赤」を成立させる。
CliRunner既定 mix_stderr=True のためresult.stderrがValueErrorになる問題。該当テストのみCliRunner(mix_stderr=False)をローカル構築(red→green 確認済み)。click の upgrade はしない。uv run ruff check96 errors → 0。挙動変更なしの機械的修正のみ(I001/E402 並べ替え、PT018 意味保存分割、RUF001/002/003 は ❯/›/× の 3 文字をallowed-confusables登記、F401 削除、F811 は意図的 re-import を noqa 保持、PLR09xx は既存慣習の noqa)。保留 3 件はレビュー裁定で解消: PT017×2 →pytest.raises標準形、B904 →raise ... from e。Verification(1:1 対応表)
packages/doeff-agents/tests/test_cli_agentd.py::test_ps_warns_about_unparseable_agentd_rows(red ログ→green、同ファイル 13/13 green)uv run ruff check→ All checks passedtest_mcp_server.py17/17 green +relaymainimport 検証CONFORMANCE_AGENTD_BIN=doeff-sessionhost)で通過Verification deviations(明示)
test_sessionhost_substrate_herdr.py5 件の既存 fail と同根)。本 PR の conformance 編集は lint 機械修正のみで、退役 Rust 既定 gate で通る範囲は基線と同一集合を維持。環境ドリフトは別 issue で追跡。test_k3_workspace_journal_e2e2 件(TYPE_CHECKING import の既存バグ)/examples/sample_entrypoints.pyの存在しないaskimport / 実 CLI e2e 3 件(router wrapper・残骸 tmux sessionmcp-live-e2e・基線 red 実証済み)。🤖 Generated with Claude Code