fix(beads-rust): use dependency_type instead of type in br show --json#308
Conversation
|
@thunter009 is attempting to deploy a commit to the plgeek Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 WalkthroughBeads-rust tracker plugin: dependency objects now use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/trackers/builtin/beads-rust/index.ts (1)
463-477: Confirmdep.typeis intentionally kept here — different CLI command, different schema.
getChildIdsparsesbr dep list --jsonoutput (typed asBrDepListItem), which usestypeper serde rename. This is correct and distinct from thebr show --jsonshape. A brief inline comment already exists on the interface (line 71), but consider adding a short note here too to prevent a future contributor from "fixing" this todependency_typeby mistake.📝 Optional: add a clarifying comment
for (const dep of deps) { - if (dep.type === 'parent-child') { + // NB: br dep list --json uses "type" (not "dependency_type" like br show --json) + if (dep.type === 'parent-child') {
da9014e to
60cf84f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/plugins/trackers/builtin/beads-rust/index.ts (1)
46-61: Interface update correctly distinguishesbr show --jsonfrombr dep list --jsonfield names.The
dependency_typefield onBrTaskJsondependencies/dependents and the retainedtypefield onBrDepListItemaccurately model the two different CLI outputs. The inline doc comments are helpful for future maintainers.One minor note:
priorityis declared as required (priority: number) on bothdependenciesanddependentsitems, but the test mock data at lines 558–577 omitspriorityfrom those objects. This won't break at runtime (JSON.parse doesn't enforce the interface), but it means tests aren't exercising the full shape. Consider makingpriorityoptional here (as it is unused on these sub-objects) or adding it to the test fixtures for consistency.
br show --json outputs dependency_type for dependencies/dependents arrays, not type. The mismatch caused dependsOn/blocks to always be empty, breaking dependency-aware status display (blocked tasks shown as actionable). Note: br dep list --json does use "type" (BrDepListItem is correct). Only BrTaskJson.dependencies/dependents were wrong. Fixes subsy#307 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
60cf84f to
521d55c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #308 +/- ##
=======================================
Coverage 44.76% 44.76%
=======================================
Files 97 97
Lines 30543 30543
=======================================
Hits 13673 13673
Misses 16870 16870
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Thanks @thunter009 - I changed this explcitly in PR 230 so investigating. |
Summary
br show --jsonoutputsdependency_typein dependencies/dependents arrays, butbrTaskToTask()checkeddep.type— field never matched, sodependsOn/blockswere always emptygetPrdContext()which filters dependents byparent-childtypeBrTaskJsoninterface to match actualbroutput (addedpriorityfield too)Note:
br dep list --jsondoes usetype— theBrDepListIteminterface was already correct. OnlyBrTaskJson.dependencies/dependentshad the mismatch.Repro
Test plan
br show --jsonoutput field name isdependency_typebr dep list --jsonoutput field name istype(unchanged)Fixes #307
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Tests