feat(skill): Anchor primitive, Task archetype, two-mode operation + validator hygiene - #5
Merged
Merged
Conversation
…tion; fix validator correctness Retarget the skill around the pipeline that consumes its output. Every story and task now names one machine-resolvable Anchor (file:line, file:symbol, module.function, METHOD /path -> status, or playwright:selector) that the held-out probe binds to. Acceptance must be runnable headless against the checkout and may not stub the thing under test; soaks and dashboards move to a Post-ship follow-up line. Add a lightweight Task archetype (Type: Bug/Chore/Spike/Ops -> Anchor / Problem / Verification) so a one-line bug is no longer inflated into a Story or filed as a one-issue Hardening ticket. Add a story Out-of-scope non-goal and a per-issue hardening Verify line. Split the flow into interactive and headless modes: headless drops the per-issue independent reviewer and the approval ceremony but keeps Gate 1 mandatory before every save (a save-hook enforces this at the tool boundary). Validator correctness: - epic-backlink demoted to WARN (Epic: none sentinel); it was the top fail-loop - labels-verify WARN deleted (undischargeable noise on every block) - vague-verb requires a metric adjacent to the verb, not anywhere on the line - code-fence and inline-code aware: quoted headings/identifiers no longer trip - --- splitting is fence-aware and merges title-less tails, so a mid-body rule can no longer drop the tail of an issue from validation - epic-story-count counts any child bullet; role-titled children exempt from em-dash - estimate is a save-time Linear field, not a body mandate; priority enum reconciled Ruff clean, 28 validator tests pass.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR updates issue-authoring guidance, validation logic, tests, and a new save-hook to support Anchor fields, the Task archetype, headless filing behavior, and stricter validator checks. ChangesAnchor/Task archetype and validator update
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant Gate1Validator as Gate1 (validate_issue.py)
participant Gate3Reviewer
participant Linear as save_issue
Agent->>Gate1Validator: run validator before save
Gate1Validator-->>Agent: pass/fail (ERROR/WARN)
alt interactive mode
Agent->>Gate3Reviewer: request independent review
Gate3Reviewer-->>Agent: approval
else headless mode
Agent->>Agent: skip per-issue Gate 3
end
Agent->>Linear: save_issue(estimate, priority)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skill/prompts/refactor-story.md (1)
94-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winShow the
Epicfield in the refactor skeleton.Step 3 already makes
**Epic:**part of the contract, includingnonefor standalone refactors, but the filing skeleton drops it. That leaves authors with no place to record the backlink the validator reads.♻️ Proposed fix
**Title:** <title> +**Epic:** <parent epic, or `none` for a deliberately standalone refactor> **Anchor:** <the new file/function being introduced> **Labels:** <domain>, tech-debt🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skill/prompts/refactor-story.md` around lines 94 - 100, The refactor skeleton is missing the Epic field even though the contract in the story template already requires it. Update the refactor story template to include an `**Epic:**` line in the filing skeleton, alongside the existing `Title`, `Anchor`, `Labels`, and `Priority` fields, so authors can provide the backlink the validator expects; keep the formatting consistent with the surrounding placeholders in the story template.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 36: Update the Task archetype row in the README so it matches the Task
contract used elsewhere by including Ops alongside Bug, Chore, and Spike. Adjust
the Task table entry in the README template reference so the allowed type list
is consistent with the rest of the docs.
- Line 5: Update the README overview paragraph to describe both archetypes in
the autonomous pipeline, not just Story: keep the existing explanation of the
planner, held-out probe, implementer, reviewer, and merge gate, but revise the
probe description so it reflects Task drafts using Problem plus Verification
alongside Story’s Requirements plus Evaluation. Use the existing overview text
near the opening paragraph and keep the wording consistent with the rest of
README.md.
In `@skill/prompts/brief-to-epic.md`:
- Around line 100-105: The story skeleton is missing the validator-readable Epic
backlink, so update the template in brief-to-epic.md to include a `**Epic:**`
line in the story body alongside `**Parent:**`. Keep `Parent` as metadata, but
restore the explicit `Epic` backlink in the `Story 1 of N` block so the
generated story shape satisfies the validator. Use the existing story header
fields (`Title`, `Anchor`, `Parent`, `Labels`) as the place to add it without
changing the overall template structure.
In `@skill/references/06-canonical-examples.md`:
- Around line 88-95: Item 1 still uses a Helicone dashboard read as a validation
gate, which conflicts with the headless Evaluation contract. Update the
acceptance criteria in the canonical examples doc so the primary check for the
relevant LLM call site is a local assertion or test, using the existing
“Validates R1 + R2” wording as the anchor. If you want to keep the dashboard
visibility check, move it into the Post-ship follow-up section alongside the 24h
soak note rather than leaving it in the main validation list.
In `@skill/scripts/tests/test_validate.py`:
- Around line 164-193: The VAGUE_WITH_ID fixture is not isolating the laundering
case because check_vague_verbs scans the entire block and the current title
already trips vague-verb before the PROJ-19 text is reached. Update the
VAGUE_WITH_ID content in test_validate.py so the title is clean and the vague
verb appears only in the body with the issue ID, or split this into a separate
fixture dedicated to the laundering path. Use the VAGUE_WITH_ID fixture and
check_vague_verbs as the main anchors while adjusting the test data.
---
Outside diff comments:
In `@skill/prompts/refactor-story.md`:
- Around line 94-100: The refactor skeleton is missing the Epic field even
though the contract in the story template already requires it. Update the
refactor story template to include an `**Epic:**` line in the filing skeleton,
alongside the existing `Title`, `Anchor`, `Labels`, and `Priority` fields, so
authors can provide the backlink the validator expects; keep the formatting
consistent with the surrounding placeholders in the story template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5481eed7-9490-4f61-8a31-b419076a64a8
📒 Files selected for processing (14)
CHANGELOG.mdREADME.mdskill/SKILL.mdskill/prompts/brief-to-epic.mdskill/prompts/hardening-issue.mdskill/prompts/refactor-story.mdskill/references/01-title-patterns.mdskill/references/02-description-templates.mdskill/references/03-tone-guide.mdskill/references/05-anti-patterns.mdskill/references/06-canonical-examples.mdskill/review/issue-reviewer.mdskill/scripts/tests/test_validate.pyskill/scripts/validate_issue.py
…boundary A headless subagent filing a batch can skip the skill's interactive Gate 1; this PreToolUse hook on mcp__linear__save_issue runs the same validator at the point of creation, and PreToolUse fires for subagent tool calls, so the batch path is covered. Create-only (updates with an id pass untouched, so the pre-Anchor corpus is never orphaned), build-lane scoped (governance labels exempt), observe by default and enforce on an operator flip via permissionDecision: deny. A missing or prose Anchor is promoted to a block on create. Fails open on bad input. Ships in the skill repo as a portable reference hook; the operator installs it and owns the enforce flip (gated on a clean corpus replay). test_hook.sh covers the 7-case decision matrix.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/test_hook.sh (1)
34-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
A && ok || badwith explicit if/then/else.Static analysis flags this pattern on lines 34-39 and 42 (SC2015):
A && B || Cis not if/then/else —Ccan also run ifBfails.ok()'s body currently never fails, so this is latent rather than an active bug, but it's fragile against future changes took()/bad().🔧 Suggested fix (apply to each assertion)
-[ "$(bad_create | decide observe)" = "allow" ] && ok || bad "observe allows bad create" +if [ "$(bad_create | decide observe)" = "allow" ]; then ok; else bad "observe allows bad create"; fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hooks/test_hook.sh` around lines 34 - 42, The shell assertions in test_hook.sh use the fragile A && ok || bad pattern, which static analysis flags as not being true if/then/else. Replace each assertion around bad_create, good_create, bad_update, missing_anchor, and governance with an explicit if/then/else block so only the intended branch runs, while keeping the existing ok and bad helpers and the observe log check unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hooks/validate-issue-on-save.sh`:
- Line 67: The exemption handling in validate-issue-on-save.sh is inconsistent:
the inline comment mentions decision/adjudication, but the default EXEMPT value
only includes human,eval, and the matching is case-sensitive. Update the EXEMPT
default or the comment so they agree, and normalize LINEAR_ISSUE_GATE_EXEMPT to
lowercase before the comparison in the exemption check logic so mixed-case
overrides still match.
---
Nitpick comments:
In `@hooks/test_hook.sh`:
- Around line 34-42: The shell assertions in test_hook.sh use the fragile A &&
ok || bad pattern, which static analysis flags as not being true if/then/else.
Replace each assertion around bad_create, good_create, bad_update,
missing_anchor, and governance with an explicit if/then/else block so only the
intended branch runs, while keeping the existing ok and bad helpers and the
observe log check unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 34ce80b2-aa72-4282-811a-54ed19ee67f5
📒 Files selected for processing (5)
CHANGELOG.mdhooks/README.mdhooks/test_hook.shhooks/validate-issue-on-save.shskill/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- skill/SKILL.md
- refactor-story + brief-to-epic filing skeletons: restore the **Epic:** backlink line the validator reads (Major: authors had no slot for it in the skeleton) - README: Task row lists Ops; overview names the Task (Problem/Verification) path alongside Story, not just Story - save-hook: lowercase LINEAR_ISSUE_GATE_EXEMPT before matching so a mixed-case override still matches the (lowercased) label names - test_hook.sh: replace the SC2015 `A && ok || bad` pattern with explicit if/then/else via an assert helper
Owner
Author
|
Addressed the review in one round (all findings were in this PR's own new code; I agree with each):
Verification after fixes: 28/28 validator tests, 7/7 hook tests, @coderabbitai resolve |
|
Resolving the review comments now. (=^･ω･^=) ノ ✅ Action performedComments resolved and changes approved. |
A corpus replay over 100 real workspace issues surfaced the one validator-defect false-fail: the old improv\w*|enhanc\w*|optimiz\w* matched noun forms, so it fired on terms of art (self-improvement, optimizer, evaluator-optimizer) and blocked three otherwise-clean build-lane issues. Restrict to improve/enhance/optimize verb and gerund forms. The replay confirmed none of the Increment-1 changes introduced any false-fail; this was pre-existing. Regression test added.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Increment 1 (+4) of the evidence-backed linear-issue-craft redesign. Retargets the skill around the autonomous pipeline that consumes its output (planner, held-out probe, implementer, reviewer, merge gate), where a false-green is the failure mode.
What changed
New authoring primitive: the Anchor. Every story and task names one machine-resolvable target (
file.ext:line,file.ext:symbol,module.function,METHOD /path -> status, orplaywright:selector) that the held-out probe binds to. A probe cannot test a thing the issue never named. WARN in the skill validator; a save-hook (next increment) promotes it to a create-only ERROR.Task archetype for a one-cause bug / chore / spike / ops change (
Type: Bug|Chore|Spike|Ops-> Anchor / Problem / Verification). Closes the coverage gap where a one-line bug had to be inflated into a Story or mis-filed as a one-issue Hardening ticket.Runnable acceptance. Evaluation items must run headless against the checkout and may not assert against a stub of the thing under test (HLB-495 shipped Done-but-broken behind a stubbed-seam eval). Soaks/dashboards move to a Post-ship follow-up line. Hardening issues gain a per-issue
Verify:line.Two-mode operation. Interactive mode keeps the three gates + approval. Headless/pipeline mode drops the per-issue reviewer and approval but keeps Gate 1 mandatory before every save (10/11 real runs were headless; one filed 80 issues with the validator never run).
Validator correctness fixes
epic-backlinkdemoted to WARN with anEpic: nonesentinel (was the feat(skill): add validate_issue.py hard gate and fix the rules it exposed #1 fail-loop)labels-verifyWARN deleted (undischargeable noise, fired 111x)## Description,robust-parser.tsno longer trip)---splitting is fence-aware and merges title-less tails (a mid-body rule can no longer drop the tail of an issue from validation)epic-story-countcounts any child bullet; role-titled children exempt from the em-dash checkVerification
ruff check .cleanpython3 skill/scripts/tests/test_validate.py-> 28 passed, 0 failed (11 original + 17 new: fence blindness,---tail-drop, standalone story, metric false-negative, Anchor shape)Full plan and evidence: 4-agent audit + war-room verdict (sequenced increments, boundary rule = issue TEXT owns what only the author knows, gates own what must hold regardless of author).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation