chore: upgrade dippin-lang to v0.17.0, add conditional node support#57
chore: upgrade dippin-lang to v0.17.0, add conditional node support#57clintecker merged 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated Go module dependency to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
clintecker
force-pushed
the
chore/dippin-lang-v0.17.0
branch
from
ed132df to
94ee223
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pipeline/dippin_adapter.go`:
- Around line 108-115: The adapter is missing a handler for ConditionalConfig
which causes FromDippinIR to return ErrUnknownConfig when dippin-lang supplies a
non-nil ConditionalConfig; add a case for ir.NodeConditional /
(*ir.ConditionalConfig) in extractNodeAttrs (it can be a no-op returning the
same attrs) so Conditional nodes are accepted, and update tests by adding
{ir.NodeConditional, "diamond", nil} to TestFromDippinIR_AllNodeKinds and ensure
TestExtractNodeAttrs_NilPointerConfig covers the nil *ir.ConditionalConfig path.
🪄 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: 9cc8712f-9981-4e69-bcc8-e34a99756590
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modpipeline/dippin_adapter.gopipeline/testdata/complex.dippipeline/testdata/simple.dip
| ir.NodeAgent: "box", // → codergen | ||
| ir.NodeHuman: "hexagon", // → wait.human | ||
| ir.NodeTool: "parallelogram", // → tool | ||
| ir.NodeParallel: "component", // → parallel | ||
| ir.NodeFanIn: "tripleoctagon", // → parallel.fan_in | ||
| ir.NodeSubgraph: "tab", // → subgraph | ||
| ir.NodeConditional: "diamond", // → conditional (pure routing, no LLM call) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only checks in repo to confirm current adapter handling and test coverage.
# 1) Confirm conditional shape mapping and config-switch coverage.
rg -nP --type=go -C3 'NodeConditional|extractNodeAttrs|ErrUnknownConfig|case\s+\*?ir\..*Config' pipeline/dippin_adapter.go
# 2) Check whether test fixtures currently exercise "conditional" nodes.
rg -nP -C2 '\bconditional\b' pipeline/testdataRepository: 2389-research/tracker
Length of output: 2523
🌐 Web query:
In github.com/2389-research/dippin-lang v0.17.0, what concrete type is stored in ir.Node.Config for nodes where Kind == ir.NodeConditional, and is it ever non-nil?
💡 Result:
No public information found on github.com/2389-research/dippin-lang or its v0.17.0 release. The repository appears private or non-existent. The tracker repo at github.com/2389-research/tracker mentions "dippin-lang" as the language for .dip pipeline files and tools like "dippin test", "dippin doctor", "dippin simulate", but provides no IR details or source code defining ir.Node.Config for NodeConditional nodes.
Citations:
- 1: https://github.com/2389-research/tracker
- 2: https://github.com/raghavnautiyal/Dip
- 3: https://www.nodesup.com
🏁 Script executed:
# Search for ConditionalConfig or similar conditional-related configs
rg -nP --type=go 'ConditionalConfig|ir\.Conditional' .
# Check for any test files or examples with conditional nodes
fd --type=go -name '*test*' pipeline/ | xargs rg -l 'conditional' 2>/dev/null
# Look for any comments or TODOs about conditional handling
rg -nP --type=go 'TODO|FIXME|conditional' pipeline/dippin_adapter.goRepository: 2389-research/tracker
Length of output: 2840
🏁 Script executed:
# Check imports in dippin_adapter.go to see what ir types are available
head -30 pipeline/dippin_adapter.go
# Search for ConditionalConfig in all files (no type restriction)
rg -n 'ConditionalConfig' .
# Look at the conditional handler to understand what config is expected
cat -n pipeline/handlers/conditional.go | head -100
# Check the adapter test file for conditional examples
rg -n -A5 -B5 'NodeConditional|conditional' pipeline/dippin_adapter_test.goRepository: 2389-research/tracker
Length of output: 2841
🏁 Script executed:
# Check go.mod to see what version of dippin-lang is required
rg -n 'dippin-lang' go.mod
# Look at the handler registry to see what handlers are registered
rg -n 'conditional' pipeline/handlers/ | head -30
# Check if there's any documentation about ConditionalNode or what config it expects
find . -name '*.md' -o -name '*.txt' | xargs rg -l -i 'conditional.*config|conditionalconfig' 2>/dev/null
# Look at how conditional nodes are created in test fixtures
rg -B10 -A10 'NodeConditional' pipeline/testdata/ 2>/dev/null | head -50Repository: 2389-research/tracker
Length of output: 1160
🏁 Script executed:
# Check what IR types are actually exported/available from dippin-lang
rg -n 'ir\.' pipeline/dippin_adapter.go | grep -E '(Config|Node)' | head -20
# Look at the full adapter test to see if conditional nodes are tested
cat -n pipeline/dippin_adapter_test.go | head -200
# Search for any test fixtures with conditional nodes
find pipeline/testdata -name '*.dip' -o -name '*.json' | xargs rg -l 'conditional' 2>/dev/nullRepository: 2389-research/tracker
Length of output: 7383
🏁 Script executed:
# Check if ConditionalConfig exists anywhere or if conditionals always have nil config
rg -n 'type.*Config struct' . --type go | grep -i conditional
# Look at what config types are defined in the test
rg -n 'ir\..*Config{}' pipeline/dippin_adapter_test.go
# Check if there's a nil config pattern for conditional nodes
rg -n 'Config: nil' pipeline/dippin_adapter_test.go
# See if conditional nodes appear anywhere with explicit config assignments
rg -B5 -A5 'NodeConditional' pipeline/dippin_adapter_test.goRepository: 2389-research/tracker
Length of output: 174
🏁 Script executed:
# Check if ConditionalConfig exists anywhere
rg 'ConditionalConfig' . --type go
# Look at the test file for NodeConditional
rg -n 'NodeConditional' pipeline/dippin_adapter_test.go
# Check test case for conditional - see what config is used
rg -B10 -A10 'NodeConditional' pipeline/dippin_adapter_test.go
# Look at all Config assignments in the test file
grep -n 'Config:' pipeline/dippin_adapter_test.goRepository: 2389-research/tracker
Length of output: 3141
🏁 Script executed:
# Search entire test file for any conditional node test
cat pipeline/dippin_adapter_test.go | grep -n 'Conditional'
# Look at the test for all node kinds more carefully - does it include conditional?
sed -n '84,149p' pipeline/dippin_adapter_test.goRepository: 2389-research/tracker
Length of output: 1704
🏁 Script executed:
# Check the nil config handling in extractNodeAttrs
sed -n '156,160p' pipeline/dippin_adapter.go
# Check if there's a test for nodes with nil config (especially conditionals)
rg -B5 -A10 'nil \*.*Config|Config: nil' pipeline/dippin_adapter_test.go | head -50Repository: 2389-research/tracker
Length of output: 1393
Add ConditionalConfig handler to extractNodeAttrs and test coverage.
The shape mapping for NodeConditional: "diamond" exists, but the adapter is missing critical infrastructure:
- No ConditionalConfig handler:
extractNodeAttrslacks a case forConditionalConfig(or pointer variant). If dippin-lang v0.17.0 provides a non-nil config,FromDippinIRwill fail withErrUnknownConfig. - No test coverage:
TestFromDippinIR_AllNodeKindsomitsNodeConditionalfrom its test cases, andTestExtractNodeAttrs_NilPointerConfigdoesn't test the nilConditionalConfigcase.
Add the missing case to the extractNodeAttrs switch (even if it's currently a no-op), and add {ir.NodeConditional, "diamond", nil} to the test suite to verify end-to-end compatibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pipeline/dippin_adapter.go` around lines 108 - 115, The adapter is missing a
handler for ConditionalConfig which causes FromDippinIR to return
ErrUnknownConfig when dippin-lang supplies a non-nil ConditionalConfig; add a
case for ir.NodeConditional / (*ir.ConditionalConfig) in extractNodeAttrs (it
can be a no-op returning the same attrs) so Conditional nodes are accepted, and
update tests by adding {ir.NodeConditional, "diamond", nil} to
TestFromDippinIR_AllNodeKinds and ensure TestExtractNodeAttrs_NilPointerConfig
covers the nil *ir.ConditionalConfig path.
There was a problem hiding this comment.
Pull request overview
Upgrades the tracker’s Dippin language dependency to dippin-lang v0.17.0 and updates internal fixtures/adapter behavior to align with upstream syntax and IR changes (including conditional “diamond” nodes).
Changes:
- Bump
github.com/2389-research/dippin-langfromv0.16.0tov0.17.0. - Update
.diptest fixtures to remove bracket edge syntax and flatten retry config. - Add
ir.NodeConditional→diamondshape mapping in the Dippin IR adapter.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pipeline/testdata/simple.dip |
Updates edge syntax away from bracket annotations. |
pipeline/testdata/complex.dip |
Updates defaults/models/fidelity, flattens retry config, and rewrites edge annotations/conditions. |
pipeline/dippin_adapter.go |
Adds conditional node kind mapping to diamond for handler compatibility. |
go.mod |
Upgrades dippin-lang to v0.17.0. |
go.sum |
Adds checksums for dippin-lang v0.17.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pipeline/testdata/complex.dip
Outdated
| approve -> done when ctx.outcome = success label: approved | ||
| approve -> init when ctx.outcome = fail label: retry restart: true |
There was a problem hiding this comment.
The routing from the human "approve" node uses when ctx.outcome = success/fail, but the tracker human handler returns outcome=success for normal choice submissions and edge selection prioritizes conditions before preferred-label routing. As a result, approve -> done when ctx.outcome = success will always win and the retry edge becomes effectively unreachable. Consider removing these when conditions and routing purely by edge labels, or change the conditions to key off ctx.preferred_label / ctx.human_response instead of ctx.outcome so the user’s selection can actually drive branching.
| approve -> done when ctx.outcome = success label: approved | |
| approve -> init when ctx.outcome = fail label: retry restart: true | |
| approve -> done label: approved | |
| approve -> init label: retry restart: true |
pipeline/testdata/complex.dip
Outdated
| human approve | ||
| label: "Approve?" | ||
| mode: choice | ||
| default: yes |
There was a problem hiding this comment.
human approve sets default: yes, but the outgoing edges are labeled approved and retry. In tracker, default maps to default_choice and is used as the preferred label; if it doesn’t match an outgoing edge label, routing can fail or fall back to non-deterministic edge selection. Update the default to match one of the outgoing edge labels (or rename the edge labels to yes/no if that’s the intended UI).
| default: yes | |
| default: approved |
| ir.NodeAgent: "box", // → codergen | ||
| ir.NodeHuman: "hexagon", // → wait.human | ||
| ir.NodeTool: "parallelogram", // → tool | ||
| ir.NodeParallel: "component", // → parallel | ||
| ir.NodeFanIn: "tripleoctagon", // → parallel.fan_in | ||
| ir.NodeSubgraph: "tab", // → subgraph | ||
| ir.NodeConditional: "diamond", // → conditional (pure routing, no LLM call) | ||
| } |
There was a problem hiding this comment.
ir.NodeConditional is now supported in the kind→shape mapping, but there isn’t an adapter test exercising this new kind. The existing TestFromDippinIR_AllNodeKinds table in pipeline/dippin_adapter_test.go is a good place to add a NodeConditional case to prevent regressions in shape/handler mapping and ensure FromDippinIR stays compatible with dippin-lang updates.
- Bump dippin-lang v0.16.0 → v0.17.0 - Add ir.NodeConditional → "diamond" shape mapping in adapter (#22) - Fix testdata/simple.dip: bracket edge syntax → when/label keywords (#18) - Fix testdata/complex.dip: nested retry block → flat attrs, bracket edge syntax → when/label keywords (#18, #3) Closes tracker-side of #18, #22, #3. #36 is linter-side only (no tracker change needed). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
clintecker
force-pushed
the
chore/dippin-lang-v0.17.0
branch
from
94ee223 to
4bc907e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| edges | ||
| start -> generate | ||
| generate -> done [label: "success"] | ||
| generate -> done label: success |
There was a problem hiding this comment.
Edge label values here are unquoted (e.g., label: success). In the rest of the repo and in README examples, edge labels are consistently written as strings (e.g., label: "approve"), which avoids ambiguity and keeps syntax consistent when labels contain spaces or reserved words. Consider quoting these edge labels (e.g., label: "success").
| generate -> done label: success | |
| generate -> done label: "success" |
Summary
Consumes all 4 upstream fixes from dippin-lang v0.17.0 (dippin-lang#3):
.dipfiles that used bracket edge syntax ([label: ...]). Parser now errors on this syntax. Migrated towhen/label:keyword syntax.ir.NodeConditional→"diamond"shape mapping in the adapter..dipfiles can now declareconditionalnodes for pure routing without LLM calls.testdata/complex.dipnested retry block → flat attrs (retry_policy,max_retries,retry_target).Test plan
go build ./...passes with dippin-lang v0.17.0go test ./... -short— all 14 packages passdippin doctor pipeline/testdata/simple.dip pipeline/testdata/complex.dip— Grade ACloses #18, #22, #3, #36
🤖 Generated with Claude Code
Summary by CodeRabbit