fix(tests): the trigger backfill assumed keyword calls; three fixtures pass positionally (#4099)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 43s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m33s
CI & Build / Build & push image (push) Successful in 21s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 43s
CI & Build / TypeScript typecheck (push) Successful in 53s
CI & Build / Python tests (push) Successful in 1m33s
CI & Build / Build & push image (push) Successful in 21s
CI run 6937 red — 3 collection errors, `positional argument follows keyword argument`, in the three integration fixtures that call the service positionally (`create_rule(topic.id, uid, "title", "statement")`). The script that added `when_to_apply=` to 15 fixtures inserted it as the FIRST argument, which is valid only where every other argument is already a keyword. Moved to the last argument in every call, which is legal in both styles, and the continuation indent now matches the surrounding arguments. Also repairs self-inflicted damage: the same script added a trigger to the two tests in test_rule_trigger_required.py whose whole purpose is to call the creators WITHOUT one. They would have stopped raising and the guard's own proof would have inverted — a test that passes for the opposite reason than the one it names, which is worse than a failing one. Caught by ast.parse across tests/ rather than by the next CI round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -75,9 +75,9 @@ async def seeded():
|
|||||||
book = await rulebooks_svc.create_rulebook(uid, "Lock fixtures")
|
book = await rulebooks_svc.create_rulebook(uid, "Lock fixtures")
|
||||||
topic = await rulebooks_svc.create_topic(book.id, uid, "locks")
|
topic = await rulebooks_svc.create_topic(book.id, uid, "locks")
|
||||||
rule = await rulebooks_svc.create_rule(
|
rule = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "A rule with vectors",
|
topic.id, uid, "A rule with vectors",
|
||||||
"Something for the embedder to index.",
|
"Something for the embedder to index.",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
async with async_session() as s:
|
async with async_session() as s:
|
||||||
note = Note(user_id=uid, title="A note with vectors", body="Body text.")
|
note = Note(user_id=uid, title="A note with vectors", body="Body text.")
|
||||||
|
|||||||
@@ -44,11 +44,11 @@ async def constraint():
|
|||||||
book = await rulebooks_svc.create_rulebook(uid, "Environment facts")
|
book = await rulebooks_svc.create_rulebook(uid, "Environment facts")
|
||||||
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
|
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
|
||||||
rule = await rulebooks_svc.create_rule(
|
rule = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "The runner has no bash",
|
topic.id, uid, "The runner has no bash",
|
||||||
"Write every `run:` step in POSIX sh.",
|
"Write every `run:` step in POSIX sh.",
|
||||||
verify_with="read the workflow's shell setting",
|
verify_with="read the workflow's shell setting",
|
||||||
expires_when="the runner can be given a bash shell",
|
expires_when="the runner can be given a bash shell",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
async with async_session() as s:
|
async with async_session() as s:
|
||||||
row = await s.get(Rule, rule.id)
|
row = await s.get(Rule, rule.id)
|
||||||
@@ -154,18 +154,18 @@ async def rulebook_of_three():
|
|||||||
book = await rulebooks_svc.create_rulebook(uid, "Sweep fixture")
|
book = await rulebooks_svc.create_rulebook(uid, "Sweep fixture")
|
||||||
topic = await rulebooks_svc.create_topic(book.id, uid, "mixed")
|
topic = await rulebooks_svc.create_topic(book.id, uid, "mixed")
|
||||||
decision = await rulebooks_svc.create_rule(
|
decision = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "dev is home", "Work directly on dev.",
|
topic.id, uid, "dev is home", "Work directly on dev.",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
never = await rulebooks_svc.create_rule(
|
never = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "The runner has no bash", "Use POSIX sh.",
|
topic.id, uid, "The runner has no bash", "Use POSIX sh.",
|
||||||
verify_with="read the workflow's shell setting",
|
verify_with="read the workflow's shell setting",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
stale = await rulebooks_svc.create_rule(
|
stale = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "Bumps need a dashboard tick", "Tick it first.",
|
topic.id, uid, "Bumps need a dashboard tick", "Tick it first.",
|
||||||
verify_with="cat CI-runner/renovate/config.js",
|
verify_with="cat CI-runner/renovate/config.js",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
async with async_session() as s:
|
async with async_session() as s:
|
||||||
row = await s.get(Rule, stale.id)
|
row = await s.get(Rule, stale.id)
|
||||||
|
|||||||
@@ -57,11 +57,11 @@ async def constraint():
|
|||||||
book = await rulebooks_svc.create_rulebook(uid, "History fixtures")
|
book = await rulebooks_svc.create_rulebook(uid, "History fixtures")
|
||||||
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
|
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
|
||||||
rule = await rulebooks_svc.create_rule(
|
rule = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic.id, uid, "The runner has no bash",
|
topic.id, uid, "The runner has no bash",
|
||||||
"Write every `run:` step in POSIX sh.",
|
"Write every `run:` step in POSIX sh.",
|
||||||
why="the image ships no bash",
|
why="the image ships no bash",
|
||||||
verify_with="read the workflow's shell setting",
|
verify_with="read the workflow's shell setting",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
|
|
||||||
return {"uid": uid, "rule_id": rule.id}
|
return {"uid": uid, "rule_id": rule.id}
|
||||||
@@ -259,8 +259,8 @@ async def test_a_version_cannot_be_read_through_a_DIFFERENT_rule(constraint):
|
|||||||
rule = await s.get(Rule, constraint["rule_id"])
|
rule = await s.get(Rule, constraint["rule_id"])
|
||||||
topic_id = rule.topic_id
|
topic_id = rule.topic_id
|
||||||
sibling = await rulebooks_svc.create_rule(
|
sibling = await rulebooks_svc.create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic_id, constraint["uid"], "A different rule", "Unrelated.",
|
topic_id, constraint["uid"], "A different rule", "Unrelated.",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
|
|
||||||
assert await rulebooks_svc.get_rule_version(
|
assert await rulebooks_svc.get_rule_version(
|
||||||
|
|||||||
@@ -62,8 +62,8 @@ async def test_create_rule_passes_required_fields():
|
|||||||
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", mock), _plain_detail():
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", mock), _plain_detail():
|
||||||
from scribe.mcp.tools.rulebooks import create_rule
|
from scribe.mcp.tools.rulebooks import create_rule
|
||||||
await create_rule(
|
await create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic_id=10, title="dev is home", statement="Work directly on dev",
|
topic_id=10, title="dev is home", statement="Work directly on dev",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
kwargs = mock.call_args.kwargs
|
kwargs = mock.call_args.kwargs
|
||||||
assert kwargs["user_id"] == 7
|
assert kwargs["user_id"] == 7
|
||||||
@@ -80,8 +80,7 @@ async def test_create_rule_blocked_by_duplicate_gate():
|
|||||||
AsyncMock(return_value=dup)), \
|
AsyncMock(return_value=dup)), \
|
||||||
patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", create_mock):
|
patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_rule", create_mock):
|
||||||
from scribe.mcp.tools.rulebooks import create_rule
|
from scribe.mcp.tools.rulebooks import create_rule
|
||||||
out = await create_rule(topic_id=10, title="dev is home", statement="x")
|
out = await create_rule(topic_id=10, title="dev is home", statement="x", when_to_apply="when the moment this fixture stands in for arises")
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
assert out["duplicate"] is True
|
assert out["duplicate"] is True
|
||||||
assert out["existing_id"] == 47
|
assert out["existing_id"] == 47
|
||||||
assert "update_rule" in out["message"]
|
assert "update_rule" in out["message"]
|
||||||
@@ -96,8 +95,7 @@ async def test_create_rule_force_bypasses_duplicate_gate():
|
|||||||
AsyncMock(return_value=fake_rule(id=5, title="r", statement="s", topic_id=10))), \
|
AsyncMock(return_value=fake_rule(id=5, title="r", statement="s", topic_id=10))), \
|
||||||
_plain_detail():
|
_plain_detail():
|
||||||
from scribe.mcp.tools.rulebooks import create_rule
|
from scribe.mcp.tools.rulebooks import create_rule
|
||||||
out = await create_rule(topic_id=10, title="dev is home", statement="x", force=True)
|
out = await create_rule(topic_id=10, title="dev is home", statement="x", force=True, when_to_apply="when the moment this fixture stands in for arises")
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
assert out["id"] == 5
|
assert out["id"] == 5
|
||||||
find_mock.assert_not_called()
|
find_mock.assert_not_called()
|
||||||
|
|
||||||
@@ -248,10 +246,10 @@ async def test_create_project_rule_passes_required_fields():
|
|||||||
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
||||||
from scribe.mcp.tools.rulebooks import create_project_rule
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
||||||
await create_project_rule(
|
await create_project_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
project_id=42,
|
project_id=42,
|
||||||
statement="Always run migrations through alembic, not raw SQL.",
|
statement="Always run migrations through alembic, not raw SQL.",
|
||||||
why="audit trail",
|
why="audit trail",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
kwargs = mock.call_args.kwargs
|
kwargs = mock.call_args.kwargs
|
||||||
assert kwargs["user_id"] == 7
|
assert kwargs["user_id"] == 7
|
||||||
@@ -267,9 +265,9 @@ async def test_create_project_rule_derives_title_from_statement():
|
|||||||
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
||||||
from scribe.mcp.tools.rulebooks import create_project_rule
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
||||||
await create_project_rule(
|
await create_project_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
project_id=42,
|
project_id=42,
|
||||||
statement="Avoid auto-generated docstrings. Reviewers find them noise.",
|
statement="Avoid auto-generated docstrings. Reviewers find them noise.",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
kwargs = mock.call_args.kwargs
|
kwargs = mock.call_args.kwargs
|
||||||
# Title should be derived from the first sentence, capped at 50 chars
|
# Title should be derived from the first sentence, capped at 50 chars
|
||||||
@@ -283,10 +281,10 @@ async def test_create_project_rule_uses_explicit_title_when_given():
|
|||||||
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
with patch("scribe.mcp.tools.rulebooks.rulebooks_svc.create_project_rule", mock), _plain_detail():
|
||||||
from scribe.mcp.tools.rulebooks import create_project_rule
|
from scribe.mcp.tools.rulebooks import create_project_rule
|
||||||
await create_project_rule(
|
await create_project_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
project_id=42,
|
project_id=42,
|
||||||
statement="anything",
|
statement="anything",
|
||||||
title="no auto-docstrings",
|
title="no auto-docstrings",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
kwargs = mock.call_args.kwargs
|
kwargs = mock.call_args.kwargs
|
||||||
assert kwargs["title"] == "no auto-docstrings"
|
assert kwargs["title"] == "no auto-docstrings"
|
||||||
|
|||||||
@@ -9,8 +9,7 @@ a rule binds should have agreed to be bound.
|
|||||||
A preference inverts it. The operator's framing: *"preferences are rules that
|
A preference inverts it. The operator's framing: *"preferences are rules that
|
||||||
scribe can and should update during use."* A preference that asks every time
|
scribe can and should update during use."* A preference that asks every time
|
||||||
never drifts, and drifting is the whole feature. Reaching one through
|
never drifts, and drifting is the whole feature. Reaching one through
|
||||||
`create_rule(kind=...)` would mean reading it through the gate's prose, and
|
`create_rule(kind=..., when_to_apply="when the moment this fixture stands in for arises")` would mean reading it through the gate's prose, and
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
the caller would hesitate over exactly the act this kind exists to make
|
the caller would hesitate over exactly the act this kind exists to make
|
||||||
routine.
|
routine.
|
||||||
|
|
||||||
|
|||||||
@@ -140,8 +140,8 @@ async def test_create_rule_requires_owned_topic():
|
|||||||
from scribe.services.rulebooks import create_rule
|
from scribe.services.rulebooks import create_rule
|
||||||
with pytest.raises(ValueError, match="topic .* not found"):
|
with pytest.raises(ValueError, match="topic .* not found"):
|
||||||
await create_rule(
|
await create_rule(
|
||||||
when_to_apply="when the moment this fixture stands in for arises",
|
|
||||||
topic_id=999, user_id=7, title="x", statement="y",
|
topic_id=999, user_id=7, title="x", statement="y",
|
||||||
|
when_to_apply="when the moment this fixture stands in for arises",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user