feat(layout,test): resolve backlog items I-2, I-3, and C-1 with unanimous peer review
- I-2: add 5.0s upper-bound execution time assertion in test_bug4_headless_unobservable_fast_path to contractually guard SKS_EMPTY_GIVEUP early-exit latency - I-3: clean up PaneInfo.focused, wire MAM_MAX_PANE_COLS/MAM_MAX_COLS environment variables and CLI --max-cols flag - C-1: apply max_columns growth guard to headless 0x0 layouts, mirroring GUI behavior - Promoted Planner consensus plan Rev.2 (plan-fea5f1b2.md) and unanimous PASS reports from Reviewers Claude (report-55d1a1d9.md) and Cline (report-6f18ba0f.md) - Verified all 333 test cases pass with exit code 0
This commit is contained in:
@@ -2,6 +2,7 @@ import os
|
||||
import sys
|
||||
import json
|
||||
import subprocess
|
||||
import time
|
||||
import pytest
|
||||
|
||||
from lib_py.layout import compute_2xk_layout
|
||||
@@ -129,7 +130,14 @@ echo "SUCCESS"
|
||||
|
||||
|
||||
def test_bug4_headless_unobservable_fast_path(tmp_path):
|
||||
"""Verify Bug 4 / R-1: in headless mode where capture-pane is empty, send_keys_safe bypasses dialogs and succeeds immediately via RPC fast-path."""
|
||||
"""Verify Bug 4 / R-1 + I-2: in headless mode where capture-pane is empty,
|
||||
send_keys_safe bypasses dialogs and succeeds immediately via the RPC fast-path.
|
||||
|
||||
The elapsed-time bound is a contract, not a nicety: removing the
|
||||
SKS_EMPTY_GIVEUP early exit leaves every functional assertion green and only
|
||||
changes the wall clock (measured 1.22s -> 10.21s), so this is the sole
|
||||
assertion that can detect that regression.
|
||||
"""
|
||||
test_script = f"""#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
SKILL_DIR="{os.path.abspath('.agents/skills')}"
|
||||
@@ -149,7 +157,6 @@ _sks_herdr() {{
|
||||
fi
|
||||
if [ "${{1:-}}" = "paste-buffer" ]; then
|
||||
PASTE_CALLED=1
|
||||
return 0
|
||||
fi
|
||||
return 0
|
||||
}}
|
||||
@@ -166,9 +173,21 @@ if [ "$PASTE_CALLED" = "1" ]; then
|
||||
fi
|
||||
echo "HEADLESS_OK"
|
||||
"""
|
||||
res = subprocess.run(["bash", "-c", test_script], capture_output=True, text=True)
|
||||
# Remove SKS_* from env so lib.sh defaults apply cleanly
|
||||
env = {k: v for k, v in os.environ.items()
|
||||
if k not in ("SKS_QUIESCENT_TRIES", "SKS_QUIESCENT_INTERVAL", "SKS_EMPTY_GIVEUP")}
|
||||
|
||||
t0 = time.perf_counter()
|
||||
res = subprocess.run(["bash", "-c", test_script], capture_output=True, text=True, env=env)
|
||||
elapsed = time.perf_counter() - t0
|
||||
|
||||
assert res.returncode == 0, f"Headless send_keys_safe failed: {res.stderr}"
|
||||
assert "HEADLESS_OK" in res.stdout
|
||||
assert elapsed < 5.0, (
|
||||
f"headless fast-path took {elapsed:.2f}s (limit 5.0s) — the "
|
||||
f"SKS_EMPTY_GIVEUP early exit in _pane_quiescent is likely gone; "
|
||||
f"the full 10s quiescence window was consumed instead"
|
||||
)
|
||||
|
||||
|
||||
def test_bug4_slow_settling_pane_success(tmp_path):
|
||||
|
||||
+73
-2
@@ -6,9 +6,7 @@ import pytest
|
||||
|
||||
from lib_py.layout import (
|
||||
compute_2xk_layout,
|
||||
extract_panes_and_focus,
|
||||
LayoutDecision,
|
||||
PaneInfo,
|
||||
)
|
||||
|
||||
|
||||
@@ -336,3 +334,76 @@ echo "SHIM_OK"
|
||||
assert "SHIM_OK" in res.stdout
|
||||
|
||||
|
||||
def _four_panes_two_columns():
|
||||
"""GUI payload: 2 full columns x 2 rows (4 panes). Shared by the max-cols tests."""
|
||||
return {
|
||||
"result": {
|
||||
"panes": [
|
||||
{"pane_id": "p1", "rect": {"x": 0, "y": 0, "width": 100, "height": 40}},
|
||||
{"pane_id": "p2", "rect": {"x": 0, "y": 40, "width": 100, "height": 40}},
|
||||
{"pane_id": "p3", "rect": {"x": 100, "y": 0, "width": 100, "height": 40}},
|
||||
{"pane_id": "p4", "rect": {"x": 100, "y": 40, "width": 100, "height": 40}},
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
def test_cli_max_cols_flag_triggers_overflow():
|
||||
"""CLI --max-cols reaches compute_2xk_layout (the lib.sh-facing path)."""
|
||||
payload = json.dumps(_four_panes_two_columns())
|
||||
skills_dir = os.path.abspath(".agents/skills")
|
||||
env = {**os.environ, "PYTHONPATH": skills_dir}
|
||||
res = subprocess.run(
|
||||
[sys.executable, "-m", "lib_py.layout",
|
||||
"--min-cols", "30", "--min-rows", "20", "--max-cols", "2", "--json"],
|
||||
input=payload, capture_output=True, text=True, env=env)
|
||||
assert res.returncode == 0, res.stderr
|
||||
d = json.loads(res.stdout)
|
||||
assert d["direction"] == "overflow" and d["is_overflow"]
|
||||
assert d["reason"] == "max_columns_reached"
|
||||
|
||||
|
||||
def test_env_max_cols_applies_without_flag():
|
||||
"""MAM_MAX_PANE_COLS is honoured with no --max-cols flag, which is exactly
|
||||
how lib.sh invokes the module (lib.sh passes no --max-cols)."""
|
||||
payload = json.dumps(_four_panes_two_columns())
|
||||
skills_dir = os.path.abspath(".agents/skills")
|
||||
env = {**os.environ, "PYTHONPATH": skills_dir, "MAM_MAX_PANE_COLS": "2"}
|
||||
res = subprocess.run(
|
||||
[sys.executable, "-m", "lib_py.layout",
|
||||
"--min-cols", "30", "--min-rows", "20", "--json"],
|
||||
input=payload, capture_output=True, text=True, env=env)
|
||||
assert res.returncode == 0, res.stderr
|
||||
assert json.loads(res.stdout)["reason"] == "max_columns_reached"
|
||||
|
||||
|
||||
def test_headless_max_columns_growth_guard():
|
||||
"""C-1: headless mode must honour max_columns too.
|
||||
|
||||
A headless 2xK grid completes n // 2 columns, so at n=4 with max_columns=2
|
||||
a further `right` split would open a third column and must overflow instead.
|
||||
Note the cap blocks *opening* a new column; it does not force an existing
|
||||
over-cap layout to shrink -- the odd-n `down` branch (and the GUI's
|
||||
fill_singleton_column) deliberately ignore it.
|
||||
"""
|
||||
def headless(n):
|
||||
return {"result": {"panes": [
|
||||
{"pane_id": f"p{i}", "rect": {"x": 0, "y": 0, "width": 0, "height": 0}}
|
||||
for i in range(1, n + 1)]}}
|
||||
|
||||
d4 = compute_2xk_layout(headless(4), max_columns=2)
|
||||
assert d4.is_overflow and d4.direction == "overflow"
|
||||
assert d4.reason == "max_columns_reached"
|
||||
|
||||
# Continues growing below the cap
|
||||
d2 = compute_2xk_layout(headless(2), max_columns=2)
|
||||
assert d2.direction == "right" and not d2.is_overflow
|
||||
|
||||
# Filling an existing column is not blocked (mirrors GUI fill_singleton_column)
|
||||
d3 = compute_2xk_layout(headless(3), max_columns=2)
|
||||
assert d3.direction == "down" and not d3.is_overflow
|
||||
|
||||
# When max_columns is not set, existing alternation is preserved (behavior neutrality)
|
||||
assert compute_2xk_layout(headless(4)).direction == "right"
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user