mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(content): reject all-filler multi-token strings in reject_trivial
The single non-empty/non-placeholder gate caught a lone banned token (wip) and below-floor strings, but multi-token soup made entirely of placeholders (wip wip, tbd / na, todo todo todo) slipped through both checks. Add an all-tokens-filler test that strips edge punctuation per token and rejects when every meaningful token is banned — without flagging real prose that merely contains a filler word (none of the tests failed). Strengthens every content model + gateway anti-soup guard that composes reject_trivial.
This commit is contained in:
@@ -14,6 +14,7 @@ Two jobs:
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import string
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
# Placeholder tokens that are never an acceptable whole-field value. Extends the
|
# Placeholder tokens that are never an acceptable whole-field value. Extends the
|
||||||
@@ -54,19 +55,36 @@ class ContentValidationError(Exception):
|
|||||||
super().__init__(f"{field}: {reason}")
|
super().__init__(f"{field}: {reason}")
|
||||||
|
|
||||||
|
|
||||||
|
def _all_tokens_filler(text: str) -> bool:
|
||||||
|
"""True when every whitespace token (sans edge punctuation) is a placeholder.
|
||||||
|
|
||||||
|
Catches multi-token soup that no single check would — ``wip wip``,
|
||||||
|
``tbd / na``, ``todo todo todo`` — without flagging real prose that merely
|
||||||
|
*contains* a filler word (``none of the tests failed``). Pure-punctuation
|
||||||
|
tokens (``/``) strip to empty and are dropped before the all-filler test.
|
||||||
|
"""
|
||||||
|
meaningful = [
|
||||||
|
stripped
|
||||||
|
for tok in text.split()
|
||||||
|
if (stripped := tok.strip(string.punctuation).lower())
|
||||||
|
]
|
||||||
|
return bool(meaningful) and all(tok in BANNED_PHRASES for tok in meaningful)
|
||||||
|
|
||||||
|
|
||||||
def reject_trivial(value: str, *, field: str, min_chars: int = 1) -> str:
|
def reject_trivial(value: str, *, field: str, min_chars: int = 1) -> str:
|
||||||
"""Return the trimmed value, or raise ``ValueError`` if it is trivial.
|
"""Return the trimmed value, or raise ``ValueError`` if it is trivial.
|
||||||
|
|
||||||
Trivial = empty, shorter than ``min_chars``, or a known placeholder token.
|
Trivial = empty, shorter than ``min_chars``, a known placeholder token, or a
|
||||||
Raises ``ValueError`` (not ``ContentValidationError``) so it can be used
|
string whose every token is a placeholder (``wip wip``). Raises
|
||||||
directly inside Pydantic field validators.
|
``ValueError`` (not ``ContentValidationError``) so it can be used directly
|
||||||
|
inside Pydantic field validators.
|
||||||
"""
|
"""
|
||||||
text = (value or "").strip()
|
text = (value or "").strip()
|
||||||
if not text:
|
if not text:
|
||||||
raise ValueError(f"{field} must not be empty")
|
raise ValueError(f"{field} must not be empty")
|
||||||
if len(text) < min_chars:
|
if len(text) < min_chars:
|
||||||
raise ValueError(f"{field} must be at least {min_chars} characters")
|
raise ValueError(f"{field} must be at least {min_chars} characters")
|
||||||
if text.lower() in BANNED_PHRASES:
|
if text.lower() in BANNED_PHRASES or _all_tokens_filler(text):
|
||||||
raise ValueError(f"{field} must not be placeholder text (got {value!r})")
|
raise ValueError(f"{field} must not be placeholder text (got {value!r})")
|
||||||
return text
|
return text
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
"""Tests for the shared content validators (``reject_trivial``).
|
||||||
|
|
||||||
|
``reject_trivial`` is the single non-empty / non-placeholder gate reused by
|
||||||
|
every structured-content field validator AND the gateway anti-soup guards. It
|
||||||
|
must reject: empty, too-short, a lone placeholder token, AND a string whose
|
||||||
|
every whitespace token is a placeholder (``wip wip``, ``tbd / na``).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from roboco.foundation.policy.content.validators import reject_trivial
|
||||||
|
|
||||||
|
|
||||||
|
def test_returns_trimmed_value_when_substantive() -> None:
|
||||||
|
assert reject_trivial(" real content here ", field="x") == "real content here"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("blank", ["", " ", "\t\n"])
|
||||||
|
def test_rejects_empty(blank: str) -> None:
|
||||||
|
with pytest.raises(ValueError, match="must not be empty"):
|
||||||
|
reject_trivial(blank, field="summary")
|
||||||
|
|
||||||
|
|
||||||
|
def test_rejects_too_short() -> None:
|
||||||
|
with pytest.raises(ValueError, match="at least 10 characters"):
|
||||||
|
reject_trivial("short", field="summary", min_chars=10)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"token", ["wip", "TBD", "asdf", "n/a", "none", "-", "...", "x"]
|
||||||
|
)
|
||||||
|
def test_rejects_lone_placeholder_token(token: str) -> None:
|
||||||
|
with pytest.raises(ValueError, match="placeholder"):
|
||||||
|
reject_trivial(token, field="summary")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"soup",
|
||||||
|
["wip wip", "tbd / na", "todo todo todo", "asdf asdf asdf asdf", "none none"],
|
||||||
|
)
|
||||||
|
def test_rejects_all_filler_token_string(soup: str) -> None:
|
||||||
|
# Every token is a placeholder — the whole string is soup even though it is
|
||||||
|
# neither a single banned token nor below the length floor.
|
||||||
|
with pytest.raises(ValueError, match="placeholder"):
|
||||||
|
reject_trivial(soup, field="summary")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"ok",
|
||||||
|
[
|
||||||
|
"none of the tests failed", # 'none' present but not all-filler
|
||||||
|
"fixed the x coordinate bug", # 'x' present but not all-filler
|
||||||
|
"rate_limited", # a real substitute reason
|
||||||
|
"LGTM, merging now",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_accepts_real_text_containing_a_filler_word(ok: str) -> None:
|
||||||
|
assert reject_trivial(ok, field="reason", min_chars=3) == ok.strip()
|
||||||
Reference in New Issue
Block a user