refactor: replace backlog-triage judgment heuristics with prompt guidance (#358) - #359
Conversation
…ance (#358) Spike Option B: scripts keep deterministic signals (mentions, merged-PR links, dates, labels); the model owns semantic judgment (blocks, depends-on, duplicates, priority and milestone proposals). - triage-relate.js: drop scanPhraseEdges/scanBlocks/scanDependsOn and title-Jaccard duplicates; emits mentions/comment-mentions/merged-pr-link - triage-stale.js: drop scanDuplicateOfClosed/closedIssueMatches/jaccard; keeps inactive/wontfix/invalid/merged-closing-pr - triage-report.js: drop rule-based priority/milestone builders; add --model-actions PATH so model-judged actions render deterministically - Rubrics: relationships.md, stale.md, decision-review.md, SKILL.md 453 tests pass / 0 fail; smoke-test 190 pass; net -454 script/test lines.
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (2)
Walkthroughbacklog-triage의 의미 기반 휴리스틱을 제거했습니다. 관계·중복·우선순위·마일스톤 판단은 모델 지침과 Changes결정론적 관계 및 stale 신호 축소
모델 액션 기반 보고서 생성
모델 액션 규칙 및 실행 연결
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ModelActions
participant TriageReport
participant DeterministicSignals
CLI->>TriageReport: --model-actions PATH
TriageReport->>ModelActions: JSON 배열 로드
ModelActions-->>TriageReport: 검증된 모델 액션
TriageReport->>DeterministicSignals: 관계·stale 신호 병합
TriageReport-->>CLI: triage 보고서와 Apply Checklist
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0b12f883d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sections = [ | ||
| { key: "classification", title: "Classification", markdown: renderClassification(snapshot) }, |
There was a problem hiding this comment.
When --model-actions contains a model-judged blocks, depends-on, or duplicate-candidate edge, this renderer still passes only the deterministic relate result to renderRelationships; loadModelActions also discards the edge's from, to, kind, and confidence fields. Consequently every semantic relationship removed from triage-relate.js is absent from the report, despite the new workflow documenting --model-actions as its replacement transport. Preserve and merge model edges into the relationship input before rendering.
AGENTS.md reference: AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
| const obsoleteActions = buildObsoleteActions(stale, { protectedIssueNumbers }); | ||
| const priorityActions = buildPriorityActions(snapshot, relate, obsoleteActions); | ||
| const milestoneActions = buildMilestoneActions(snapshot, relate, obsoleteActions, priorityActions); | ||
| const allActions = [...obsoleteActions, ...priorityActions, ...milestoneActions]; | ||
| const priorityActions = modelActions.filter((action) => action.section === "priority"); | ||
| const milestoneActions = modelActions.filter((action) => action.section === "milestone"); | ||
| const allActions = dedupeActions([...obsoleteActions, ...priorityActions, ...milestoneActions]); |
There was a problem hiding this comment.
When the model identifies an open issue as a duplicate of a closed issue, the advertised replacement flow cannot surface that proposal: obsoleteActions is built solely from deterministic stale output, while model actions are accepted only for priority and milestone. An obsolete/close-duplicate model action is silently dropped from both Obsolete Candidates and the Apply Checklist, so removing scanDuplicateOfClosed eliminates this capability rather than transferring its judgment to the model. Merge validated obsolete model actions here while retaining active-sprint protection.
AGENTS.md reference: AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@skills/backlog-triage/scripts/triage-report.js`:
- Around line 598-607: Validate model actions before the mapping in the triage
report flow, enforcing allowed section/verb combinations, positive safe-integer
issueNumber values, plain-object args, and each section’s required arguments.
Reject unsupported or malformed actions with an error before rendering or
including them in priorityActions and the Apply Checklist, and add a CLI test
covering an invalid model action file.
- Around line 560-566: Model relationship edges and duplicate-of-closed obsolete
proposals are currently dropped from the report. In
skills/backlog-triage/scripts/triage-report.js:560-566, define and load separate
model-action schemas through loadModelActions and merge relationship actions
into the existing Relationships path and obsolete proposals into Obsolete
Candidates. Document the supported edge and proposal formats in
skills/backlog-triage/references/relationships.md:60-89 and
skills/backlog-triage/references/stale.md:18-36, limit claims to implemented
behavior in skills/backlog-triage/SKILL.md:71-79, add JSON and markdown contract
coverage in skills/backlog-triage/scripts/triage-report.test.js:376-419, and
align the completion wording in CHANGELOG.md:11.
🪄 Autofix
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 Plus
Run ID: d7400dac-1143-4f77-8919-1c4ad241a7f9
📒 Files selected for processing (12)
CHANGELOG.mdskills/backlog-triage/SKILL.mdskills/backlog-triage/references/classification.mdskills/backlog-triage/references/decision-review.mdskills/backlog-triage/references/relationships.mdskills/backlog-triage/references/stale.mdskills/backlog-triage/scripts/triage-relate.jsskills/backlog-triage/scripts/triage-relate.test.jsskills/backlog-triage/scripts/triage-report.jsskills/backlog-triage/scripts/triage-report.test.jsskills/backlog-triage/scripts/triage-stale.jsskills/backlog-triage/scripts/triage-stale.test.js
💤 Files with no reviewable changes (1)
- skills/backlog-triage/scripts/triage-stale.js
Address review feedback on PR #359 (claude-opus-5 + codex gpt-5.6-sol, both NOT-LGTM): - loadModelActions now validates every entry: allowed section/verb combos, positive safe-integer issueNumber (or from/to for relationship edges), plain-object args, per-section required args, non-empty summary; malformed entries throw with the array index - --model-actions gains section "relationship" (edges merge into Relationships) and section "obsolete" (close/revisit/close-duplicate merge into Obsolete Candidates), so the docs' claims now match behavior and merge-into has a producer again - dedupeActions keys match the apply contract (verb, issueNumber, args) instead of section:verb:issueNumber - priority/milestone sections render evidence; renderObsoleteCandidates shows model actions even without a stale input - docs: SKILL.md phase model + JSON examples, README/workflow-patterns recipes, CHANGELOG line count; drop now-inert duplicate_threshold (lib.js, triage-config.yml, collect/lib tests) - tests: loadModelActions file loading + 7 rejection cases, relationship/ obsolete merge, apply-contract dedupe 464 tests pass / 0 fail; smoke-test 190 pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e17c01886e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| const requiredArgs = section === "priority" ? ["value"] : section === "milestone" ? ["name"] : []; |
There was a problem hiding this comment.
When a model-generated close-duplicate action omits args.target or supplies an empty value, this validator accepts it because obsolete actions have no required payload fields. If the proposal is checked, triage-apply.js posts a malformed Duplicate of . comment and then invokes gh issue close on the issue, leaving a destructively closed issue without its duplicate target; reject such actions before rendering.
Useful? React with 👍 / 👎.
| } else { | ||
| if (!Number.isSafeInteger(action.issueNumber) || action.issueNumber <= 0) { | ||
| throw new Error(`${label} must carry a positive safe-integer issueNumber.`); | ||
| } | ||
| } |
There was a problem hiding this comment.
When the model hallucinates an issue number or swaps the open duplicate with a closed target, any positive integer passes this check. The resulting accepted anchor can mutate or close an unrelated issue in the repository, whereas the removed builders could only create actions while iterating snapshot.issues; cross-check non-relationship issueNumber values against the snapshot before rendering actionable anchors.
Useful? React with 👍 / 👎.
| issueNumber: action.section === "relationship" ? undefined : action.issueNumber, | ||
| args: action.args, | ||
| cluster: typeof action.cluster === "string" ? action.cluster : undefined, | ||
| sprintName: typeof action.sprintName === "string" ? action.sprintName : undefined, |
There was a problem hiding this comment.
A milestone action with the required args.name but no top-level sprintName passes validation and is normalized here with sprintName: undefined. renderMilestoneSuggestions groups on that field, so otherwise valid model output renders under a literal ### undefined heading; require sprintName or default it from args.name.
Useful? React with 👍 / 👎.
Round-2 review (claude-opus-5 + codex gpt-5.6-sol, both NOT-LGTM): - validateModelAction now rejects null/empty-string required args (priority->value, milestone->name), requires reason for obsolete close and target+reason for close-duplicate, and requires a top-level sprintName for milestone grouping (prevents '### undefined' headings and a localeCompare crash that would abort the whole report) - dedupe key now uses the same normalizeArgs/stableSerialize as triage-apply.js (deferred require to avoid the circular dependency), so key order and whitespace no longer cause duplicate anchors - references/relationships.md and stale.md now document the wire shape the loader actually accepts (section/verb/args wrapper) instead of raw edges or the old suggested_action grammar - drop the inert duplicate_threshold remnants from the report test fixture 467 tests pass / 0 fail; smoke-test 190 pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ff2f4556b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const action of actions) { | ||
| const key = `${action.section}:${action.verb}:${action.issueNumber}`; | ||
| const key = normalizeActionKey(action); | ||
| const current = selected.get(key); |
There was a problem hiding this comment.
When one issue satisfies multiple stale signals—for example, a wontfix issue that is also older than stale_days—each generated close action has different reason args, so this full apply-key dedupe retains every proposal. If the reviewer checks both, triage-apply.js executes two comment-and-close command pairs because its idempotency key also includes the args; this can duplicate comments or fail on the second close and stop later accepted actions. Preserve one winning close proposal per issue/verb while using the full key only to dedupe repeated render surfaces.
Useful? React with 👍 / 👎.
| sprintName: typeof action.sprintName === "string" ? action.sprintName : undefined, | ||
| summary: String(action.summary), | ||
| evidence: typeof action.evidence === "string" ? action.evidence : undefined, |
There was a problem hiding this comment.
When model output supplies a summary that disagrees with its verb or args, validation accepts it and this value becomes the human-facing checkbox text while triage-apply.js executes the anchor payload. For example, an action can display “Set priority:high” but carry args.value: "low", so checking the advertised proposal performs a different mutation. Construct summaries from the validated payload, or reject summaries that do not match it, so the confirmation surface cannot misrepresent the accepted action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/backlog-triage/SKILL.md (1)
110-144: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win모델 액션의 공통 필수 필드를 문서화하세요.
제공된
triage-report.js, Lines 645-704는 모든 섹션에 비어 있지 않은summary를 요구하고,milestone액션에는 최상위sprintName을 요구합니다. 현재 Line 110은relationship액션의summary와 milestone 액션의 최상위sprintName을 명시하지 않습니다. 모델이 이 설명만 따르면 보고서 검증에서 거부될 수 있습니다. 모든 액션의 공통 필드와 milestone 전용 필드를 문장으로 명시하세요.🤖 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 `@skills/backlog-triage/SKILL.md` around lines 110 - 144, Update the model-judged action documentation to state that every action requires a non-empty top-level summary, and that milestone actions additionally require the top-level sprintName field. Ensure the relationship example and the milestone field requirements match the validation enforced by triage-report.js.
🧹 Nitpick comments (1)
skills/backlog-triage/scripts/triage-report.js (1)
697-699: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value조건식을 괄호로 명확하게 표현하세요.
Line 697은
&&와||를 괄호 없이 혼합합니다. 연산자 우선순위 때문에 동작은 의도와 같지만, 의도를 읽기 어렵습니다. 두 번째 검사는 첫 번째 검사를 포함하므로 하나의 조건으로 줄일 수 있습니다.♻️ 제안 리팩터
- if (section === "milestone" && typeof action.sprintName !== "string" || section === "milestone" && !String(action.sprintName || "").trim()) { + if (section === "milestone" && (typeof action.sprintName !== "string" || action.sprintName.trim() === "")) { throw new Error(`${label} (assign-milestone) requires a non-empty top-level sprintName for grouping.`); }🤖 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 `@skills/backlog-triage/scripts/triage-report.js` around lines 697 - 699, Update the validation condition in the milestone handling branch to group the section check once and combine it with the sprintName validity checks using explicit parentheses. Preserve the requirement that section is "milestone" and action.sprintName must be a non-empty string, while removing the redundant repeated section comparison.
🤖 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 `@skills/backlog-triage/scripts/triage-report.js`:
- Around line 580-582: Apply dedupeActions to the priorityActions and
milestoneActions results in the report generation flow, alongside the existing
obsoleteActions handling, before rendering section-specific lists. Preserve the
existing allActions aggregation while ensuring each section removes duplicate
actions using the same (verb, issueNumber, args) criteria.
- Around line 611-624: Update mergeModelRelationships so that after appending
model relationships to base.edges, it sorts the merged edges using the same
ordering as renderRelationships and removes duplicate edges according to
triage-relate.js’s edgeIdentity(...) criteria. Preserve the existing edge
normalization and relate object handling.
In `@skills/backlog-triage/scripts/triage-report.test.js`:
- Around line 822-849: Update the test using buildReportModel so both duplicate
set-priority actions use a section permitted for that verb by
MODEL_SECTION_VERBS, while retaining the same (verb, issueNumber, args) values
and dedupe assertion. Ensure the fixture represents valid loadModelActions input
and still verifies only one apply entry.
In `@skills/backlog-triage/SKILL.md`:
- Around line 28-29: Update the model-judgment descriptions in the
Report/Analyze, model-action guidance, and Report/Render sections to explicitly
include obsolete actions. State that model-generated obsolete actions are
combined with deterministic stale signals by triage-report.js, while preserving
the existing descriptions of relationship, priority, and milestone decisions.
---
Outside diff comments:
In `@skills/backlog-triage/SKILL.md`:
- Around line 110-144: Update the model-judged action documentation to state
that every action requires a non-empty top-level summary, and that milestone
actions additionally require the top-level sprintName field. Ensure the
relationship example and the milestone field requirements match the validation
enforced by triage-report.js.
---
Nitpick comments:
In `@skills/backlog-triage/scripts/triage-report.js`:
- Around line 697-699: Update the validation condition in the milestone handling
branch to group the section check once and combine it with the sprintName
validity checks using explicit parentheses. Preserve the requirement that
section is "milestone" and action.sprintName must be a non-empty string, while
removing the redundant repeated section comparison.
🪄 Autofix
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 Plus
Run ID: ef19f264-8a12-46f1-b415-e5c88f7c7531
📒 Files selected for processing (14)
CHANGELOG.mdREADME.mdbacklog/triage-config.ymlskills/backlog-triage/SKILL.mdskills/backlog-triage/references/relationships.mdskills/backlog-triage/references/stale.mdskills/backlog-triage/scripts/triage-collect.test.jsskills/backlog-triage/scripts/triage-relate.jsskills/backlog-triage/scripts/triage-relate.test.jsskills/backlog-triage/scripts/triage-report.jsskills/backlog-triage/scripts/triage-report.test.jsskills/dev-backlog/references/workflow-patterns.mdskills/dev-backlog/scripts/lib.jsskills/dev-backlog/scripts/lib.test.js
💤 Files with no reviewable changes (4)
- backlog/triage-config.yml
- skills/backlog-triage/scripts/triage-collect.test.js
- skills/dev-backlog/scripts/lib.js
- skills/backlog-triage/scripts/triage-relate.js
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- skills/backlog-triage/references/stale.md
- skills/backlog-triage/references/relationships.md
| const base = relate && Array.isArray(relate.edges) | ||
| ? { ...relate, edges: [...relate.edges] } | ||
| : { edges: [] }; | ||
|
|
||
| for (const action of modelRelationships) { | ||
| const edge = action.args; | ||
| base.edges.push({ | ||
| from: edge.from, | ||
| to: edge.to, | ||
| kind: edge.kind, | ||
| confidence: Number.isFinite(edge.confidence) ? edge.confidence : 1, | ||
| evidence: typeof edge.evidence === "object" && edge.evidence !== null ? edge.evidence : {}, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# renderRelationships와 sortEdges의 구현 및 relate 객체 사용 필드를 확인
fd -t f 'triage-relate.js|triage-report.js' skills/backlog-triage/scripts --exec ast-grep outline {} --items all
rg -nP -C 8 'function renderRelationships|function sortEdges' skills/backlog-triage/scripts
rg -nP -C 3 '\brelate\.(?!edges)\w+' skills/backlog-triage/scriptsRepository: sungjunlee/dev-backlog
Length of output: 9866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== triage-report.js relevant sections =="
sed -n '1,220p' skills/backlog-triage/scripts/triage-report.js
sed -n '220,380p' skills/backlog-triage/scripts/triage-report.js
sed -n '600,665p' skills/backlog-triage/scripts/triage-report.js
echo
echo "== triage-relate.js relevant sections =="
sed -n '1,190p' skills/backlog-triage/scripts/triage-relate.js
sed -n '270,340p' skills/backlog-triage/scripts/triage-relate.js
echo
echo "== all relate field references =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' '\brelate\b|\.\b(relate|generated|kindEtc|extra|model)\b' skills/backlog-triage 2>/dev/null | head -200Repository: sungjunlee/dev-backlog
Length of output: 30243
병합 후 엣지를 정렬하고 같은 엣지를 제거하세요.
renderRelationships는 relate.edges를 정렬하고 렌더링하지만, mergeModelRelationships는 모델 엣지를 끝에만 추가합니다. 결정론적이게 만들려면 병합한 배열 다시 정렬하고, triage-relate.js의 edgeIdentity(...) 기준으로 동등한 엣지를 제거해야 합니다.
현재 relate 객체에는 edges만 참조되므로 generated 같은 필드가 깨질 우려는 없습니다.
🤖 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 `@skills/backlog-triage/scripts/triage-report.js` around lines 611 - 624,
Update mergeModelRelationships so that after appending model relationships to
base.edges, it sorts the merged edges using the same ordering as
renderRelationships and removes duplicate edges according to triage-relate.js’s
edgeIdentity(...) criteria. Preserve the existing edge normalization and relate
object handling.
| | Report | Analyze | Classification and deterministic signals come from scripts; the model judges blocks/depends-on/duplicates, priority, and milestone actions from the snapshot and writes them to a `--model-actions` JSON file. Alignment and Decision Review are prompt-driven from the same evidence. | | ||
| | Report | Render | `triage-report.js` validates the model actions, merges them with deterministic signals, and writes one markdown report with anchored proposals and a consolidated Apply Checklist. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
obsolete 모델 액션을 분석 및 판단 분포에 명시하세요.
제공된 triage-report.js, Lines 567-606은 section === "obsolete" 모델 액션을 결정론적 stale 액션과 병합합니다. 그러나 Lines 28-29, 71-80, 148-152는 모델 판단을 관계, 우선순위, 마일스톤으로만 설명합니다. 이 불일치는 모델이 유효한 obsolete 액션을 생성하지 않게 할 수 있습니다. 세 설명에 obsolete 모델 액션과 결정론적 stale 신호의 관계를 명시하세요.
Also applies to: 71-80, 148-152
🤖 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 `@skills/backlog-triage/SKILL.md` around lines 28 - 29, Update the
model-judgment descriptions in the Report/Analyze, model-action guidance, and
Report/Render sections to explicitly include obsolete actions. State that
model-generated obsolete actions are combined with deterministic stale signals
by triage-report.js, while preserving the existing descriptions of relationship,
priority, and milestone decisions.
Round-3 review (both reviewers NOT-LGTM on a single P1): - move module.exports above the main() invocation so the CLI path populates exports before dedupeActions' deferred require of triage-apply.js; verified no warnings on a real CLI run - dedupe priority/milestone section arrays too so the report stays internally consistent with the Apply Checklist - simplify the milestone sprintName check - test: add a dedupe case for args differing only by key order and whitespace (locks in normalizeArgs/stableSerialize parity with apply) 468 tests pass / 0 fail; smoke-test 190 pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c6cea41ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const key of requiredArgs) { | ||
| const value = args[key]; | ||
| if (typeof value !== "string" || value.trim() === "") { | ||
| throw new Error(`${label} (${verb}) requires a non-empty string arg "${key}".`); | ||
| } |
There was a problem hiding this comment.
When a model emits a multiline mutation argument—for example, a reason containing a newline—this validation accepts it, but formatAnchor() inserts the newline literally into the HTML anchor. Since parseReport() passes one line at a time to parseAnchor(), neither copy of that anchor is recognized, so checking the visibly rendered proposal never applies the action. Reject or normalize line breaks in anchor-bound model arguments before rendering.
Useful? React with 👍 / 👎.
Round-4 review: codex flagged P1 (numeric sprintName passed validation then coerced to undefined, rendering a literal '### undefined' milestone group); claude agreed as a suggestion. Restore the typeof check in validateModelAction and add regression tests: - reject numeric sprintName for milestone actions - dedupe two same-key actions within the priority section itself (locks section-level dedupe, not just the Apply Checklist) 470 tests pass / 0 fail; smoke-test 190 pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d16ca94a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # model-authored: blocks/depends-on/duplicate edges + priority/milestone actions | ||
| node $SKILL/triage-report.js --snapshot $SNAP --relate /tmp/relate.json --stale /tmp/stale.json --model-actions /tmp/model-actions.json |
There was a problem hiding this comment.
In the documented workflow, triage-collect.js is still invoked without --with-closed-issues, so the snapshot contains no closed_issues evidence even though this new model step advertises duplicate edges and replaces duplicate-of-closed detection. Because the skill defines the snapshot as the canonical input and forbids downstream re-fetches, following this command sequence cannot produce duplicate-of-closed proposals; add the collection flag to this workflow (and the matching workflow-patterns example) or narrow the advertised scope to open/open duplicates.
Useful? React with 👍 / 👎.
| confidence: Number.isFinite(edge.confidence) ? edge.confidence : 1, | ||
| evidence: typeof edge.evidence === "object" && edge.evidence !== null ? edge.evidence : {}, |
There was a problem hiding this comment.
When a documented model duplicate-candidate supplies evidence.reason but omits the optional confidence, this conversion invents confidence 1; the existing duplicate renderer then ignores reason and prints only score 1.00 because it still expects the removed Jaccard overlap evidence. The resulting report hides the model's actual rationale and presents an unsupported maximum score, so translate the new semantic evidence into the rendered line rather than defaulting it to the old heuristic shape.
Useful? React with 👍 / 👎.
| const requiredArgs = section === "priority" | ||
| ? ["value"] | ||
| : section === "milestone" |
There was a problem hiding this comment.
When the model emits a typo or any value other than the rubric's required high, this validation accepts it as long as it is non-empty. If the proposal is checked, triage-apply.js constructs and adds priority:<value> while removing the existing priority label; this either fails on a repository without that label and stops later accepted actions, or applies a priority outside the advertised proposal semantics. Validate args.value === "high" before rendering the anchor.
Useful? React with 👍 / 👎.
Claude round-5 suggestion: cluster had the same validate/load asymmetry as sprintName — a non-string cluster passed validation, was coerced to undefined, and silently collapsed into the uncategorized group. Reject a non-string cluster when present so the optional-field surface is uniform. 470 tests pass / 0 fail; smoke-test 190 pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57d2d382a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modelRelationships = modelActions.filter((action) => action.section === "relationship"); | ||
| const mergedRelate = mergeModelRelationships(relate, modelRelationships); | ||
|
|
||
| const priorityActions = dedupeActions(modelActions.filter((action) => action.section === "priority")); |
There was a problem hiding this comment.
When a snapshot issue already has priority:critical, a model-generated set-priority action with the documented value high is retained here even though the proposal rubric excludes critical issues. If checked, triage-apply.js removes the existing priority label and adds priority:high, silently downgrading the issue; the removed buildPriorityActions explicitly skipped both high and critical issues, so retain that snapshot-based eligibility check when accepting model actions.
Useful? React with 👍 / 👎.
| const mergedRelate = mergeModelRelationships(relate, modelRelationships); | ||
|
|
||
| const priorityActions = dedupeActions(modelActions.filter((action) => action.section === "priority")); | ||
| const milestoneActions = dedupeActions(modelActions.filter((action) => action.section === "milestone")); |
There was a problem hiding this comment.
When the model proposes assign-milestone for an issue that already has a milestone in the snapshot, this accepts and renders the action without checking current state. Checking it causes triage-apply.js to replace the existing milestone, despite the documented model rubric requiring an unassigned issue; the removed milestone builder skipped every issue whose milestone was non-null, so preserve that guard for model actions.
Useful? React with 👍 / 👎.
| const priorityActions = dedupeActions(modelActions.filter((action) => action.section === "priority")); | ||
| const milestoneActions = dedupeActions(modelActions.filter((action) => action.section === "milestone")); |
There was a problem hiding this comment.
When deterministic stale analysis proposes closing an issue but the model also emits a priority or milestone action for it, these unfiltered lists retain all proposals. If the reviewer checks them, apply can close the issue and then relabel or milestone that closed issue; the removed builders used the obsolete-action set to exclude such issues, matching the new rubric's “not already proposed for close” requirement, so filter both lists against closing obsolete actions before rendering.
Useful? React with 👍 / 👎.
1 participant
Summary
Spike Option B (measure-first, user-approved): replace ~265 judgment/heuristic lines in
backlog-triagescripts with prompt-level rubric guidance. Deterministic computation stays in scripts; semantic judgment moves to the model.Changes
scripts/triage-relate.jsblocks/depends-on(scanPhraseEdges/scanBlocks/scanDependsOn) and title-Jaccard duplicates (tokenizeTitle/jaccardSimilarity/findDuplicateCandidates). Script now emits deterministic edges only:mentions,comment-mentions,merged-pr-link. Dropped unusedreadTriageConfig/resolveBacklogDir/DEFAULT_CONFIG_PATH.scripts/triage-stale.jsscanDuplicateOfClosed/closedIssueMatches/jaccardSimilarity/tokenizeTitleandSIGNALS.DUPLICATE_OF_CLOSED. Keeps deterministic date/label signals:inactive,wontfix,invalid,merged-closing-pr.scripts/triage-report.jsbuildThemeStats/buildRelationshipCounts/buildPriorityActions/buildMilestoneActions(andgetIsoWeek/nextSprintName). Added--model-actions PATH— the model's judged priority/milestone actions are rendered deterministically (anchors, dedupe, Apply Checklist unchanged).SKILL.md+references/×4--model-actionsJSON format, judgment-distribution principle, and rubrics for model-judged blocks/depends-on/duplicate/priority/milestone.CHANGELOG.md[Unreleased]→ Removed entry. Closes #358.Judgment distribution
--model-actionsJSON so the report's machine contract never changes.Verification
Summary by CodeRabbit
새 기능
개선 사항