diff --git a/docs/skill-triggers.md b/docs/skill-triggers.md index d6f707a..31bd0c4 100644 --- a/docs/skill-triggers.md +++ b/docs/skill-triggers.md @@ -57,11 +57,11 @@ instead share vocabulary with its `description:`. -### Auto-fire (21) +### Auto-fire (22) The model may invoke these from a description match. Everything here is a gate or a procedure that is useless if it only runs when named. -`create-skill`, `draft-pr`, `make-pr`, `thrash-reflect-automate`, `principle-flag-your-own-corrections`, `principle-prove-it`, `principle-subagent-inherits-scope`, `prove-it-ship-gate`, `alternatives-considered`, `diu`, `how`, `land-stack`, `loop-generator`, `narrow-the-scope`, `plan-first`, `ship-a-detector`, `show-me-your-work`, `spike-and-validate`, `split-scope`, `visual-proof`, `why` +`create-skill`, `draft-pr`, `make-pr`, `phrase-judge`, `thrash-reflect-automate`, `principle-flag-your-own-corrections`, `principle-prove-it`, `principle-subagent-inherits-scope`, `prove-it-ship-gate`, `alternatives-considered`, `diu`, `how`, `land-stack`, `loop-generator`, `narrow-the-scope`, `plan-first`, `ship-a-detector`, `show-me-your-work`, `spike-and-validate`, `split-scope`, `visual-proof`, `why` ### Explicit invocation only (32) diff --git a/engine/hooks/diu-stop/phrases/plain-words-code-names.json b/engine/hooks/diu-stop/phrases/plain-words-code-names.json new file mode 100644 index 0000000..85b9f69 --- /dev/null +++ b/engine/hooks/diu-stop/phrases/plain-words-code-names.json @@ -0,0 +1,17 @@ +{ + "checker": "plain-words-code-names", + "meaning": "The reply puts a raw code, config, or function name in front of the user as if it were a word, without saying in everyday words what it does.", + "reads": "exchange", + "match": [ + "Set disable-model-invocation: true on the skill.", + "It flips desiredEnabled: true.", + "materialize_revision now calls realpath.", + "The check reads EXIT_CODES and PULL_REF_RE." + ], + "not_match": [ + "The skill is hidden from the automatic skill list, so only a typed command can start it.", + "The worker is switched on.", + "The replay tool now follows folder shortcuts before comparing paths (backtest_detector.py:102)." + ], + "on_hit": "plain-words: the last reply named code or settings without saying what they do in everyday words." +} diff --git a/engine/hooks/diu-stop/phrases/plain-words-internal-names.json b/engine/hooks/diu-stop/phrases/plain-words-internal-names.json new file mode 100644 index 0000000..2f0201c --- /dev/null +++ b/engine/hooks/diu-stop/phrases/plain-words-internal-names.json @@ -0,0 +1,18 @@ +{ + "checker": "plain-words-internal-names", + "meaning": "The reply names an internal tool, worker, queue, or process step as if the user already knows what it is, without saying what it does.", + "reads": "exchange", + "match": [ + "It points at a real design gap in e2e-autofix.", + "Run the invoker-watcher first.", + "The backlog grew after the redeploy before it started shrinking.", + "Preflight passes and the review unit is engine-runtime.", + "The merge gate is review_ready." + ], + "not_match": [ + "The worker that retries failed browser tests is making many copies of one job.", + "The repo's pre-publish check passes.", + "You asked about the merge queue: it is the line of PRs waiting for checks before they merge." + ], + "on_hit": "plain-words: the last reply named an internal tool or step without saying what it does." +} diff --git a/engine/hooks/diu-stop/phrases/plain-words-made-up-labels.json b/engine/hooks/diu-stop/phrases/plain-words-made-up-labels.json new file mode 100644 index 0000000..cc21570 --- /dev/null +++ b/engine/hooks/diu-stop/phrases/plain-words-made-up-labels.json @@ -0,0 +1,18 @@ +{ + "checker": "plain-words-made-up-labels", + "meaning": "The reply uses a label the assistant made up while working, which the user has not used, and does not say in everyday words what it means.", + "reads": "exchange", + "match": [ + "No hook decides differently.", + "The hooks only gain replay functions.", + "Is this the right safety line?", + "That was the fake problem.", + "Each slice passes on its own." + ], + "not_match": [ + "The hook's logic does not change; this is a simple refactor.", + "Each hook gets a small extra function that replays old chats; the live hook never calls it.", + "You asked about the safety line: it is one sentence saying why the change cannot break anything." + ], + "on_hit": "plain-words: the last reply used a made-up label; say what it means in everyday words or drop it." +} diff --git a/engine/hooks/diu-stop/phrases/plain-words-status-words.json b/engine/hooks/diu-stop/phrases/plain-words-status-words.json new file mode 100644 index 0000000..c018486 --- /dev/null +++ b/engine/hooks/diu-stop/phrases/plain-words-status-words.json @@ -0,0 +1,18 @@ +{ + "checker": "plain-words-status-words", + "meaning": "The reply uses a status word or state label without saying what actually happened or why.", + "reads": "exchange", + "match": [ + "Three tests were capped.", + "The reset is blocked.", + "That would double count.", + "Only a restart picks up newer code.", + "The branch is stale." + ], + "not_match": [ + "Three tests hit the retry limit, so they stopped and now need a person.", + "The restart could not run because a check refused it.", + "The running program still uses the version it loaded earlier; a restart loads the current version." + ], + "on_hit": "plain-words: the last reply used a status word without saying what happened." +} diff --git a/engine/hooks/diu-stop/phrases/plain-words-tech-jargon.json b/engine/hooks/diu-stop/phrases/plain-words-tech-jargon.json new file mode 100644 index 0000000..7e0e6ab --- /dev/null +++ b/engine/hooks/diu-stop/phrases/plain-words-tech-jargon.json @@ -0,0 +1,18 @@ +{ + "checker": "plain-words-tech-jargon", + "meaning": "The reply uses technical jargon or an abbreviation the user has not used, without a plain explanation next to it.", + "reads": "exchange", + "match": [ + "It died on a 401 unauthorized.", + "That is a remote infra gap.", + "The script ships as an SEA.", + "Mostly regex false positives.", + "Force-push with lease, then a three-way apply." + ], + "not_match": [ + "The login had expired, so the server refused the request.", + "The problem is on the other machines, not this laptop.", + "Most of the alarms were wrong: the word matched but the meaning did not." + ], + "on_hit": "plain-words: the last reply used jargon or an abbreviation without a plain explanation." +} diff --git a/engine/hooks/diu-stop/tests/test_plain_words.py b/engine/hooks/diu-stop/tests/test_plain_words.py new file mode 100644 index 0000000..9b7a2d3 --- /dev/null +++ b/engine/hooks/diu-stop/tests/test_plain_words.py @@ -0,0 +1,75 @@ +import os +import sys +import unittest + +HOOK_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +LLM_JUDGE_DIR = os.path.join(os.path.dirname(HOOK_DIR), "llm-judge") +sys.path.insert(0, LLM_JUDGE_DIR) + +import phrases + +PHRASES_DIR = os.path.join(HOOK_DIR, "phrases") +PREFIX = "plain-words-" +CATEGORIES = ( + "plain-words-made-up-labels", + "plain-words-code-names", + "plain-words-internal-names", + "plain-words-tech-jargon", + "plain-words-status-words", +) +EDGE = ".,;:()!?\"'" + + +def _is_pr_number(token): + return token.startswith("#") and token.strip(EDGE).lstrip("#").isdigit() + + +def _is_date(token): + parts = token.strip(EDGE).split("-") + return len(parts) == 3 and len(parts[0]) == 4 and all(part.isdigit() for part in parts) + + +def _load(checker): + return phrases.load(checker, directory=PHRASES_DIR) + + +class TestPlainWords(unittest.TestCase): + def test_every_category_file_is_listed(self): + found = sorted(name[:-5] for name in os.listdir(PHRASES_DIR) if name.startswith(PREFIX) and name.endswith(".json")) + self.assertEqual(found, sorted(CATEGORIES)) + + def test_each_category_loads_and_reads_the_exchange(self): + for checker in CATEGORIES: + with self.subTest(checker=checker): + dictionary = _load(checker) + self.assertEqual(dictionary["reads"], "exchange") + self.assertGreaterEqual(len(dictionary["match"]), 3) + self.assertGreaterEqual(len(dictionary["not_match"]), 2) + self.assertTrue(dictionary["on_hit"].startswith("plain-words:")) + + def test_no_phrase_names_a_pr_number_or_a_date(self): + for checker in CATEGORIES: + dictionary = _load(checker) + for phrase in [dictionary["meaning"], *dictionary["match"], *dictionary["not_match"]]: + for token in phrase.split(): + with self.subTest(checker=checker, token=token): + self.assertFalse(_is_pr_number(token)) + self.assertFalse(_is_date(token)) + + def test_no_phrase_sits_in_two_categories(self): + seen = {} + for checker in CATEGORIES: + dictionary = _load(checker) + for phrase in dictionary["match"] + dictionary["not_match"]: + self.assertNotIn(phrase, seen, f"{phrase!r} is in {seen.get(phrase)} and {checker}") + seen[phrase] = checker + + def test_a_pr_number_and_a_date_are_recognised(self): + self.assertTrue(_is_pr_number("#412.")) + self.assertTrue(_is_date("2026-09-11")) + self.assertFalse(_is_pr_number("#tag")) + self.assertFalse(_is_date("engine-runtime")) + + +if __name__ == "__main__": + unittest.main() diff --git a/engine/skills/make-pr/SKILL.md b/engine/skills/make-pr/SKILL.md index 1cf0f62..42246f6 100644 --- a/engine/skills/make-pr/SKILL.md +++ b/engine/skills/make-pr/SKILL.md @@ -27,6 +27,32 @@ along. See [docs/ecosystem.md](../../../docs/ecosystem.md). A change whose code reads another unit's output is two stacked PRs, producer first -- not one PR with the coupling explained in Slice Rationale. +## Stack on top, or overwrite the branch + +A PR that already exists gets one of two treatments, decided from its current +diff against its base -- never from how much work went into it, and never by +asking the user which they would prefer: + +- **Stack on top** when the claim still holds and what is being added is + small: a fix to code the PR already ships, the base merged in, or docs for + the same claim. Push another commit to the same branch, or open a new PR + based on it. +- **Overwrite the branch** when the diff no longer matches the claim. Any one + of these is enough: + - it carries work already on the base + - it mixes review units + - it ships a second claim + + Save the old head as `backup/pr-`, say where it went, + then `git push --force-with-lease=:` and rewrite the title + and body to the slice that remains. + +The record that gets read later is the PR's final diff and body, so a branch +whose diff stopped matching its title is rewritten, not explained in Slice +Rationale. `--force-with-lease` and the backup branch are the floor, not a +precaution to skip: a lease failure means another session moved that branch, +so re-read it and decide again instead of forcing past it. + ## Preflight (run first) ```sh diff --git a/engine/skills/make-pr/tests/test_stack_or_overwrite.py b/engine/skills/make-pr/tests/test_stack_or_overwrite.py new file mode 100644 index 0000000..8cba408 --- /dev/null +++ b/engine/skills/make-pr/tests/test_stack_or_overwrite.py @@ -0,0 +1,42 @@ +import os +import unittest + +SKILL = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "SKILL.md") +HEADING = "## Stack on top, or overwrite the branch" + + +class TestStackOrOverwrite(unittest.TestCase): + @classmethod + def setUpClass(cls): + with open(SKILL, encoding="utf-8") as handle: + cls.text = handle.read() + cls.section = cls.text.split(HEADING, 1)[-1].split("\n## ", 1)[0] + + def test_the_skill_has_the_section(self): + self.assertIn(HEADING, self.text) + + def test_the_section_names_both_treatments(self): + self.assertIn("**Stack on top**", self.section) + self.assertIn("**Overwrite the branch**", self.section) + + def test_stacking_is_for_a_claim_that_still_holds(self): + self.assertIn("the claim still holds", self.section) + + def test_overwriting_is_for_a_diff_that_stopped_matching_the_claim(self): + for trigger in ("work already on the base", "mixes review units", "ships a second claim"): + with self.subTest(trigger=trigger): + self.assertIn(trigger, self.section) + + def test_overwriting_requires_a_lease_and_a_backup_branch(self): + self.assertIn("--force-with-lease", self.section) + self.assertIn("backup/pr-", self.section) + + def test_a_lease_failure_stops_the_overwrite(self): + self.assertIn("re-read it and decide again", self.section) + + def test_the_choice_is_not_a_question_for_the_user(self): + self.assertIn("never by\nasking the user", self.section) + + +if __name__ == "__main__": + unittest.main()