diff --git a/docs/Tuning-Guide.md b/docs/Tuning-Guide.md index 38c4532..37165d8 100644 --- a/docs/Tuning-Guide.md +++ b/docs/Tuning-Guide.md @@ -645,7 +645,7 @@ How long a new head's review waits while its checks are pending. A review judges the CI result (R1), and waiting keeps it off a GPU that CI is using (the `nisse` project's end-to-end tests). Finished checks, passed or failed, end the wait; after this long it reviews anyway and the CI result reads as -pending. +pending. 0 never waits. - **Too low:** reviews run beside CI, slower on a shared GPU, without its result. - **Too high:** a stuck runner delays reviews that long. @@ -656,7 +656,8 @@ pending. How long a new head with no checks at all waits for some to appear: Gitea Actions reports a push's checks a moment after it. Past this, a repository -without CI is reviewed at once. +without CI is reviewed at once. 0 reviews such a head at once (the live +review test, whose fresh repository has no CI). - **Too low:** a review starts before CI has reported, and runs beside it. - **Too high:** repositories without CI wait that long for every review. diff --git a/nisse-core/src/nisse_core/config.py b/nisse-core/src/nisse_core/config.py index aaa59eb..d17d93f 100644 --- a/nisse-core/src/nisse_core/config.py +++ b/nisse-core/src/nisse_core/config.py @@ -81,13 +81,14 @@ def read_file(path: Path, errors: list[str]) -> dict[str, Any]: return loaded -def positive(value: Any, kind: type) -> int | float | None: +def positive(value: Any, kind: type, *, zero: bool = False) -> int | float | None: + """``value`` as ``kind`` if above 0 (or 0 itself, given ``zero``).""" # bool is an int in Python; `true` for a turn limit is a mistake. if isinstance(value, bool) or not isinstance(value, int | float): return None if kind is int and not float(value).is_integer(): return None - if value <= 0: + if value < 0 or (value == 0 and not zero): return None return kind(value) @@ -132,9 +133,13 @@ def parse_tuning( if spec is None: errors.append(f"tuning.{name}.{key}: unknown setting") continue - number = positive(value, float if spec.type in (float, "float") else int) + # A field with metadata {"zero": True} may also be 0 ("don't wait"). + zero = bool(spec.metadata.get("zero")) + kind = float if spec.type in (float, "float") else int + number = positive(value, kind, zero=zero) if number is None: - errors.append(f"tuning.{name}.{key}: must be a positive number") + need = "zero or more" if zero else "a positive number" + errors.append(f"tuning.{name}.{key}: must be {need}") continue values[key] = number sections[name] = cls(**values) diff --git a/nisse-core/tests/test_shared_config.py b/nisse-core/tests/test_shared_config.py index 4ad585e..234904d 100644 --- a/nisse-core/tests/test_shared_config.py +++ b/nisse-core/tests/test_shared_config.py @@ -19,6 +19,7 @@ from nisse_core.config import ( class Review: max_rounds: int = 3 wait_s: float = 60.0 + grace_s: float = field(default=5.0, metadata={"zero": True}) @dataclass(frozen=True) @@ -45,6 +46,17 @@ def test_an_agents_own_tuning_sections() -> None: ] +def test_a_field_may_opt_in_to_zero() -> None: + errors: list[str] = [] + tuning = parse_tuning({"review": {"grace_s": 0}}, Tuning, errors) + assert tuning.review.grace_s == 0 and errors == [] + parse_tuning({"review": {"grace_s": -1, "wait_s": 0}}, Tuning, errors) + assert errors == [ + "tuning.review.grace_s: must be zero or more", + "tuning.review.wait_s: must be a positive number", + ] + + def test_references_and_people() -> None: errors: list[str] = [] env = {"TOKEN": "s3cret"} diff --git a/nisse-review/src/nisse_review/config.py b/nisse-review/src/nisse_review/config.py index e69bd5f..0bb0f46 100644 --- a/nisse-review/src/nisse_review/config.py +++ b/nisse-review/src/nisse_review/config.py @@ -56,9 +56,9 @@ class ReviewTuning: diff_chars: int = 60000 # the diff in the review's context; files on demand attempts: int = 2 # failed runs on one PR before saying so on it # A new head waits for its CI to finish: at most ci_wait_s while checks - # are pending, and ci_grace_s for checks to appear at all. - ci_wait_s: float = 3600.0 - ci_grace_s: float = 120.0 + # are pending, and ci_grace_s for checks to appear at all. 0: don't wait. + ci_wait_s: float = field(default=3600.0, metadata={"zero": True}) + ci_grace_s: float = field(default=120.0, metadata={"zero": True}) @dataclass(frozen=True) diff --git a/nisse-review/tests/test_review.py b/nisse-review/tests/test_review.py index 8785ff4..ed6f91a 100644 --- a/nisse-review/tests/test_review.py +++ b/nisse-review/tests/test_review.py @@ -431,6 +431,21 @@ def test_a_repository_pattern_must_be_owner_slash_name(tmp_path: Path) -> None: ReviewConfig.from_tree({**tree, "repositories": [bad]}, {}) +def test_the_ci_waits_may_be_zero(tmp_path: Path) -> None: + # The live test's repository has no CI: it reviews with no grace at all. + from nisse_review.config import ReviewConfig + + tree = { + "state_db": str(tmp_path / "s.db"), + "llm": {"base_url": "http://llm", "token": "k"}, + "roles": {"reviewer": "local", "expert": "frontier"}, + "gitea": {"url": "https://g", "token": "t", "operators": ["don"]}, + "tuning": {"review": {"ci_wait_s": 0, "ci_grace_s": 0}}, + } + review = ReviewConfig.from_tree(tree, {}).tuning.review + assert (review.ci_wait_s, review.ci_grace_s) == (0, 0) + + def test_diff_lines_are_the_heads_lines_a_diff_shows() -> None: diff = ( "--- a/x.py\n+++ b/x.py\n@@ -1,3 +1,4 @@\n a\n-b\n+c\n+d\n e\n"