副作用分類からヒューリスティクスを撤廃する - #115
Merged
Merged
Conversation
`--read-only` refused any command not classified "read", and that
classification came from `_is_read_command`, which pattern-matched command-name
suffixes (" get", " info", " list", ...). A write command whose name happened to
end in " get" would have silently slipped through the read-only gate, and no
test could catch it -- the tests could only check that names followed the
convention. `_DESTRUCTIVE_COMMANDS` had the mirror problem: a hand-maintained
frozenset far from the command definitions, where an omission silently
downgraded a destructive command to "write" with no confirmation prompt.
All 250 public commands now declare their side effect in `_SIDE_EFFECTS`, and
`_side_effect_spec` raises for anything undeclared rather than defaulting.
Tests pin the table to `public_command_names()` in both directions.
Mechanically generated from the previous classification: identical output, and
the public contract snapshot is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ects # Conflicts: # tests/test_command_registry.py
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
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.
--read-onlyの保証がコマンド名の語尾に依存していた。この戻り値が
read_only_remote_command_names()を経て_client_core._dispatch()の--read-onlyゲートになる。" get"で終わる書き込みコマンドが追加された瞬間に--read-onlyは静かに破れ、テストでは「命名規約に従っているか」しか検証できない。_DESTRUCTIVE_COMMANDSも同じ問題を持つ。コマンド定義から離れた手書き frozenset で、分類漏れは検出されずwrite(requires_confirmation=False)として黙って扱われる。変更
_is_read_command()を削除(未使用関数を残さない)。_DESTRUCTIVE_COMMANDSfrozenset を削除。_SIDE_EFFECTSに明示宣言。_side_effect_spec()は未宣言コマンドに対して 例外を投げる。デフォルト値を返さないため、宣言漏れがあればcommand_specs()の呼び出し自体が落ちて CI が止まる。差分ゼロの確認
移行前の分類を機械的に書き出し、移行後の
command_specs()出力とプログラムで突合して 完全一致を確認済み。tests/snapshots/public_contract_snapshot.jsonは無変更。分類が変わったコマンドは なし。idempotentは現状「read なら True」の一律であり、本 PR では意図的に見直していない(分類変更と表現形式変更を混ぜると差分ゼロの検証ができなくなるため)。transport tempo setなどの個別見直しは後続の別 PR で 1 件ずつ根拠を添えて行う。指示からの逸脱 1 点(レビュー希望)
テーブルは 1 行ごとに
SideEffectSpec(...)を書き下すのではなく、共有の frozen 定数 3 つ(_READ/_WRITE/_DESTRUCTIVE)を使っている。現行の分類は実測で ちょうど 3 通り(
write136 /read88 /destructive26)しか存在しないため無損失であり、1 行ごとの構築子展開は DRY 原則と重複検出閾値の両方に逆らう。コマンドごとの宣言が明示的である点は変わらない。範囲外だが記録した依存
public_command_names()がcommands/を正規表現で走査している問題(レビュー項目 2.2)は本 PR では直していない。ただし grep が壊れるとテーブルの網羅性検証も同時に無意味になるという依存関係をcommand_specs.pyの該当箇所にコメントとして明記し、2.3 の descriptor 化で両方が解消される見込みであることも書き添えた。テスト
public_command_names()の全要素が_SIDE_EFFECTSに存在すること(両方向)_side_effect_spec()を呼ぶと例外になることkind="destructive"⇒requires_confirmation=True、kind="read"⇒idempotent=Truesynth {type} {keys|set|observe}等)が個別に宣言されていること--read-onlyで到達可能な remote コマンドに非 read が混ざらないこと公開契約への影響
なし(意図的にゼロ)。
検証
dev_checks/ruff check/ruff format --check/pytest(1019 passed)/check_remote_script_runtime.py/generate_skill_docs.py+git diff --exit-code/update_public_contract_snapshot.py(差分なし)。品質ハーネスは pristine main の時点で既に exit 1(baseline 陳腐化 + 既存 fail 1 件)。本 PR はテーブル 250 行を追加しても fail-level violation 集合を main と一致に保ち、警告も 1 件も増やしていない。
Ableton Live 手動検証
不要。純粋な機械的移行であり Live の挙動に触れない。
🤖 Generated with Claude Code