chore: configure union merge strategy for agent workflow files #250
Conversation
Configures AGENTS.md, docs/decision-log.md, and TASKS-DAG.md to use the 'union' merge driver. This automatically resolves conflicts by accepting lines from both sides, which is appropriate for these append-only log files.
Extends the union merge strategy to include docs/execution-plan.md, ensuring that daily intent logs and task updates can be merged from multiple branches without conflict.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds task-dependency infrastructure: a Markdown-based TASKS-DAG, scripts to parse and render it to DOT/SVG, supporting documentation, .gitattributes merge rules, and updates to working-agreement workflow requirements (isolated branches and PR closing/CI checks). Changes
Sequence DiagramsequenceDiagram
actor Dev as Developer
participant Script as generate-tasks-dag.js
participant FS as Filesystem
participant Parser as Markdown Parser
participant DotGen as DOT Generator
participant Graphviz as Graphviz (dot)
Dev->>Script: run node scripts/generate-tasks-dag.js
Script->>FS: read `TASKS-DAG.md`
FS-->>Script: markdown content
Script->>Parser: parseTasksDag(content)
Parser-->>Script: nodes, edges, metadata
Script->>DotGen: generateDot(nodes, edges, clusters, attrs)
DotGen-->>Script: DOT text
Script->>FS: write `docs/assets/dags-2/tasks-dag.dot`
alt Graphviz available
Script->>Graphviz: spawn `dot -Tsvg` on DOT
Graphviz-->>FS: write `tasks-dag.svg`
FS-->>Script: ✓ SVG generated
else Graphviz missing
Graphviz--xScript: render error
Script-->>Dev: warn (DOT still produced)
end
Script-->>Dev: complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.gitattributes
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-03T02:01:23.896Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T02:01:23.896Z
Learning: Update `docs/execution-plan.md` and `docs/decision-log.md` when a PR touches non-doc code
Applied to files:
.gitattributes
| AGENTS.md merge=union | ||
| docs/decision-log.md merge=union | ||
| TASKS-DAG.md merge=union | ||
| docs/execution-plan.md merge=union No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Ensure file ends with a newline (Git best practice).
.gitattributes files should end with a trailing newline. Verify the file has a newline after line 4 to align with POSIX standards and Git conventions.
🤖 Prompt for AI Agents
.gitattributes lines 1-4: the file is missing a trailing newline at EOF; add a
single newline character after line 4 so the file ends with a trailing newline
(POSIX/Git convention) and save the file to ensure the final byte is a newline.
Missing rationale documentation and decision log entry.
This .gitattributes configuration creates a permanent constraint on these four files (append-only with no deletions). Without inline documentation or a decision log entry:
- Future maintainers won't understand why union merge exists or when it can be violated
- Reviewers won't know to enforce append-only constraints
- There's no audit trail for this structural decision
Per your team's learning, update docs/decision-log.md to document:
- Why union merge was chosen for these specific files
- The strict append-only contract these files must maintain
- What happens if this constraint is violated (data duplication/corruption)
- Which files are safe to modify normally vs. append-only
Would you like me to generate a decision log entry documenting the union merge rationale, constraints, and risks?
🤖 Prompt for AI Agents
In .gitattributes lines 1-4 the union merge setting was added for AGENTS.md,
docs/decision-log.md, TASKS-DAG.md, and docs/execution-plan.md but there is no
decision-log entry or inline rationale; add a new entry to docs/decision-log.md
that states why union merge was chosen for these specific files, declares the
strict append-only contract (no deletions or reordering), describes the
risks/impact if violated (duplication/corruption and merge conflicts), and
enumerates which files are append-only versus which can be edited normally so
future maintainers and reviewers can enforce the rule.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== File existence check ==="
for file in AGENTS.md docs/decision-log.md TASKS-DAG.md docs/execution-plan.md; do
if [ -f "$file" ]; then
echo "✓ $file exists ($(wc -l < "$file") lines)"
else
echo "✗ $file NOT FOUND"
fi
done
echo -e "\n=== .gitattributes EOF check ==="
if [ -f .gitattributes ]; then
if [ -z "$(tail -c 1 .gitattributes)" ]; then
echo "✓ .gitattributes has proper EOF newline"
else
echo "✗ .gitattributes missing EOF newline"
fi
echo "Content:"
cat -A .gitattributes
fi
echo -e "\n=== Git history for deletions (checking each file) ==="
for file in AGENTS.md docs/decision-log.md TASKS-DAG.md docs/execution-plan.md; do
echo "--- $file ---"
git log --oneline -- "$file" 2>/dev/null | head -5 || echo "No git history"
echo "Deletions in diff:"
git log -p -- "$file" 2>/dev/null | grep -c "^-" | head -1 || echo "0"
doneRepository: flyingrobots/echo
Length of output: 857
🏁 Script executed:
#!/bin/bash
echo "=== Checking if decision-log.md was updated in this PR ==="
git diff HEAD~1 docs/decision-log.md 2>/dev/null || echo "Could not find recent diff"
echo -e "\n=== Verifying union merge behavior documentation ==="
git config merge.union || echo "Union merge driver not explicitly configured"
echo -e "\n=== Listing all tracked .md files ==="
git ls-files "*.md" | head -20Repository: flyingrobots/echo
Length of output: 819
🏁 Script executed:
#!/bin/bash
echo "=== Checking for any glob patterns or missing entries ==="
# Check if there are other .md files that should potentially have the merge=union setting
echo "All markdown files in tracked index:"
git ls-files "*.md" "docs/*.md" 2>/dev/null | sort
echo -e "\n=== .gitattributes completeness ==="
echo "Files with merge=union configured:"
grep "merge=union" .gitattributes 2>/dev/null
echo -e "\n=== Checking if .gitattributes file itself is correct ==="
wc -l .gitattributes
od -c .gitattributes | tail -3Repository: flyingrobots/echo
Length of output: 4286
Four critical configuration errors requiring immediate fixes.
-
TASKS-DAG.md does not exist — It's configured in
.gitattributesbut the file is absent from the repository. This entry will be silently ignored by git and represents incomplete configuration. -
Missing POSIX EOF newline in .gitattributes — Lines 1–3 lack trailing newlines. This violates
.gitattributesconventions and will cause parsing inconsistencies. Add a proper newline after the final entry. -
Union merge strategy requires documented append-only contract — The
merge=uniondriver will unconditionally accept lines from both conflict sides. If any of these files ever undergo deletions or reorganization, the union driver reintroduces deleted lines, causing data corruption. Without explicit documentation that these files are permanently append-only with zero mutations allowed, this configuration is hazardous. Add inline.gitattributescomments explaining the append-only invariant and risks. -
Decision log not updated — Per repository practice,
docs/decision-log.mdmust be updated when PRs touch configuration files. This PR modifies.gitattributes(non-doc code) but leaves the decision log untouched. Add a dated entry documenting the union merge strategy decision, rationale, and append-only constraints.
🤖 Prompt for AI Agents
In .gitattributes around lines 1 to 4: fix four issues — (1) remove or correct
the TASKS-DAG.md entry if the file does not exist (either add TASKS-DAG.md to
the repo if it should exist, or delete the line from .gitattributes), (2) ensure
each line ends with a POSIX newline (add a trailing newline after the final
entry), (3) add an inline comment above each merge=union line documenting the
append-only contract and risks (explicitly state these files must be
append-only, no deletions or reorders, and that union may reintroduce removed
lines), and (4) add a dated entry to docs/decision-log.md describing the
decision to use merge=union for these files, the rationale, and the required
append-only constraints; update the PR to include the created/edited
TASKS-DAG.md or removal of its .gitattributes entry, the newline fix, the inline
comments, and the decision-log update.
Adds TASKS-DAG.md (auto-generated issue dependencies) and a new visualization pipeline: - scripts/generate-tasks-dag.js: Parses TASKS-DAG.md to DOT/SVG. - docs/assets/dags-2/: Output directory for the new graph. - docs/dependency-dags-2.md: Documentation page displaying the result. This allows comparing the 'inferred' dependencies from issue bodies against the planned dependencies in docs/dependency-dags.md.
Corrects the regex for parsing TASKS-DAG.md issue links and filters out isolated nodes to produce a readable dependency graph.
Explicitly states that every new objective (task, feature, fix) must begin on a fresh branch based on main, preventing unrelated changes from being mixed in a single PR.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
docs/assets/dags-2/tasks-dag.dotis excluded by!**/*.dotdocs/assets/dags-2/tasks-dag.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
TASKS-DAG.mddocs/dependency-dags-2.mdscripts/generate-tasks-dag.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-03T02:01:23.896Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T02:01:23.896Z
Learning: Applies to docs/assets/dags/** : Planning DAG artifacts live in `docs/assets/dags/` and should be documented in `docs/dependency-dags.md`; set `DAG_REFRESH_ISSUE=<issue-number>` for automated DAG refresh PRs
Applied to files:
TASKS-DAG.mdscripts/generate-tasks-dag.jsdocs/dependency-dags-2.md
🪛 LanguageTool
TASKS-DAG.md
[style] ~325-~325: Try using a descriptive adverb here.
Context: ...controlled desync lessons (make it fail on purpose)](https://github.com/flyingrobots/echo/...
(ON_PURPOSE_DELIBERATELY)
[style] ~345-~345: Try using a descriptive adverb here.
Context: ...controlled desync lessons (make it fail on purpose)](https://github.com/flyingrobots/echo/...
(ON_PURPOSE_DELIBERATELY)
🪛 markdownlint-cli2 (0.18.1)
TASKS-DAG.md
8-8: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
47-47: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
50-50: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
56-56: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
59-59: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
62-62: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
65-65: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
74-74: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
77-77: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
89-89: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
95-95: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
101-101: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
107-107: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
113-113: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
119-119: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
125-125: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
131-131: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
134-134: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
137-137: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
140-140: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
143-143: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
146-146: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
149-149: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
152-152: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
155-155: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
158-158: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
161-161: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
164-164: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
167-167: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
170-170: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
173-173: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
176-176: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
179-179: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
182-182: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
185-185: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
188-188: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
191-191: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
194-194: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
197-197: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
219-219: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
235-235: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
241-241: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
244-244: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
247-247: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
250-250: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
253-253: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
256-256: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
259-259: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
262-262: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
265-265: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
268-268: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
271-271: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
274-274: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
277-277: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
280-280: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
283-283: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
289-289: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
292-292: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
295-295: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
301-301: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
307-307: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
310-310: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
313-313: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
319-319: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
325-325: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
331-331: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
337-337: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
352-352: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
361-361: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
374-374: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
387-387: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
397-397: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
403-403: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
409-409: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
415-415: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
439-439: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
442-442: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
448-448: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
454-454: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
460-460: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
464-464: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (10)
docs/dependency-dags-2.md (2)
13-19: Path convention deviation from established patterns.Per learnings, DAG artifacts typically reside in
docs/assets/dags/and are documented indocs/dependency-dags.md. This PR introduces a parallel structure withdocs/assets/dags-2/documented indocs/dependency-dags-2.md. While this separation may be intentional (distinguishing auto-generated from manually curated DAGs), ensure this divergence is deliberate and documented.Based on learnings: DAG artifacts live in
docs/assets/dags/per established conventions.
23-31: Documentation is clear and accurate.The regeneration instructions correctly reference the script and document the required dependencies (Node.js and Graphviz).
scripts/generate-tasks-dag.js (7)
1-22: Utility functions are well-implemented.The
fail()andrunChecked()functions properly handle errors and use safe APIs.spawnSyncwith array arguments prevents command injection vulnerabilities.
24-108: Parsing logic is sound with proper edge case handling.The parser correctly:
- Handles forward references by creating placeholder nodes
- Manages pending edges across section transitions
- Pushes the final pending edge after the loop
The logic properly sequences edge metadata collection and handles mode switching cleanly.
110-121: String escaping and attribute mapping are correct.
escapeDotStringproperly escapes both backslashes and quotes for DOT format.confidenceAttrsprovides sensible visual distinctions for dependency confidence levels with a safe fallback.
123-135: Cluster name extraction heuristic is reasonable.The prefix matching logic properly categorizes issues into clusters with a sensible "Misc" fallback. The
.replace(":", "")safely handles both colon-suffixed and plain prefixes.
137-151: Isolated node filtering is correct.The logic properly identifies connected nodes by collecting all IDs that appear in edges, then filters the node map accordingly. This successfully removes isolated nodes from the output as intended.
153-159: DOT graph configuration is well-structured.The header uses modern Graphviz features (newrank, splines) and sensible defaults for layout and appearance. All syntax is valid.
184-187: Label wrapping heuristic is imprecise but acceptable.The length check at line 184 operates on the escaped string length (which includes
\\and\"sequences), not the visual character count. The regex at line 186 only wraps the first long word sequence. For typical issue titles this is adequate, but extremely long titles may still overflow.Given this is visualization code with "good enough" as the bar, this is acceptable.
TASKS-DAG.md (1)
1-464: The markdown formatting claims in this review are unfounded. Verification reveals:
No MD022 violations: The file properly includes blank lines after headers. The markdownlint config enforces
"blank_lines": true, and the actual file structure complies (blank line after each## [#...]header before section content).No MD047 violation: The file ends with a trailing newline, so there is no missing final newline.
Wrong generator identified:
scripts/generate-tasks-dag.jsis a reader that parses TASKS-DAG.md to generate visualization files (DOT → SVG). It doesn't write TASKS-DAG.md. No script was found that writes this file—it appears to be manually maintained.One legitimate issue: The
issueRegexvariable on line 33 ofscripts/generate-tasks-dag.jsis unused and should be removed, but this is a minor linting issue in a visualization script, not related to TASKS-DAG.md formatting.The review's core premise—that the generator is producing malformed markdown—is incorrect. TASKS-DAG.md already conforms to the markdownlint rules currently enforced.
Likely an incorrect or invalid review comment.
| // SPDX-License-Identifier: Apache-2.0 | ||
| // © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots> | ||
|
|
||
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
| import { spawnSync } from "node:child_process"; | ||
|
|
||
| const INPUT_FILE = "TASKS-DAG.md"; | ||
| const OUT_DIR = "docs/assets/dags-2"; | ||
| const DOT_FILE = path.join(OUT_DIR, "tasks-dag.dot"); | ||
| const SVG_FILE = path.join(OUT_DIR, "tasks-dag.svg"); | ||
|
|
||
| function fail(message) { | ||
| process.stderr.write(`${message}\n`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| function runChecked(cmd, args) { | ||
| const result = spawnSync(cmd, args, { encoding: "utf8" }); | ||
| if (result.error) fail(`Failed to run ${cmd}: ${result.error.message}`); | ||
| if (result.status !== 0) fail(`Command failed: ${cmd} ${args.join(" ")}\n${result.stderr}`); | ||
| } | ||
|
|
||
| function parseTasksDag(content) { | ||
| const lines = content.split("\n"); | ||
| const nodes = new Map(); // number -> { number, title, url } | ||
| const edges = []; // { from, to, confidence, note } | ||
|
|
||
| let currentIssue = null; | ||
| let mode = null; // 'blocks' or 'blocked_by' | ||
|
|
||
| const issueRegex = /^## \[#(\d+): (.+)](.+)"/; | ||
| const linkRegex = /^\s+- \[#(\d+): (.*?)\]\((.*)\)/; | ||
| const confidenceRegex = /^\s+- Confidence: (.+)/; | ||
| const evidenceRegex = /^\s+- Evidence: (.+)/; | ||
|
|
||
| let pendingEdge = null; | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith("## [")) { | ||
| const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/); | ||
| if (issueMatch) { | ||
| if (pendingEdge) { edges.push(pendingEdge); pendingEdge = null; } | ||
| const number = parseInt(issueMatch[1], 10); | ||
| currentIssue = { number, title: issueMatch[2], url: issueMatch[3] }; | ||
| nodes.set(number, currentIssue); | ||
| mode = null; | ||
| continue; | ||
| } else { | ||
| console.warn("Failed to match header:", line); | ||
| } | ||
| } | ||
|
|
||
| if (!currentIssue) continue; | ||
|
|
||
| // Section Headers | ||
| if (line.trim() === "- Blocked by:") { | ||
| mode = "blocked_by"; | ||
| if (pendingEdge) { edges.push(pendingEdge); pendingEdge = null; } | ||
| continue; | ||
| } | ||
| if (line.trim() === "- Blocks:") { | ||
| mode = "blocks"; | ||
| if (pendingEdge) { edges.push(pendingEdge); pendingEdge = null; } | ||
| continue; | ||
| } | ||
|
|
||
| // Dependency Link | ||
| const linkMatch = line.match(linkRegex); | ||
| if (linkMatch) { | ||
| if (pendingEdge) { edges.push(pendingEdge); pendingEdge = null; } | ||
| const targetNumber = parseInt(linkMatch[1], 10); | ||
| const targetTitle = linkMatch[2]; | ||
| const targetUrl = linkMatch[3]; | ||
|
|
||
| // Ensure target node exists (even if we haven't reached its header yet) | ||
| if (!nodes.has(targetNumber)) { | ||
| nodes.set(targetNumber, { number: targetNumber, title: targetTitle, url: targetUrl }); | ||
| } | ||
|
|
||
| if (mode === "blocked_by") { | ||
| // Target -> Current | ||
| pendingEdge = { from: targetNumber, to: currentIssue.number, confidence: "strong", note: "" }; | ||
| } else if (mode === "blocks") { | ||
| // Current -> Target | ||
| pendingEdge = { from: currentIssue.number, to: targetNumber, confidence: "strong", note: "" }; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // Metadata for the pending edge | ||
| if (pendingEdge) { | ||
| const confMatch = line.match(confidenceRegex); | ||
| if (confMatch) { | ||
| pendingEdge.confidence = confMatch[1].trim().toLowerCase(); | ||
| continue; | ||
| } | ||
| const evMatch = line.match(evidenceRegex); | ||
| if (evMatch) { | ||
| pendingEdge.note = evMatch[1].trim(); | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
| if (pendingEdge) { edges.push(pendingEdge); } | ||
|
|
||
| return { nodes, edges }; | ||
| } | ||
|
|
||
| function escapeDotString(str) { | ||
| return String(str).replace(/\\/g, "\\\\").replace(/"/g, '\\"'); | ||
| } | ||
|
|
||
| function confidenceAttrs(confidence) { | ||
| switch (confidence) { | ||
| case "strong": return 'color="black", penwidth=1.4, style="solid"'; | ||
| case "medium": return 'color="gray40", penwidth=1.2, style="dashed"'; | ||
| case "weak": return 'color="gray70", penwidth=1.2, style="dotted"'; | ||
| default: return 'color="black", style="solid"'; // fallback | ||
| } | ||
| } | ||
|
|
||
| // Heuristic to guess cluster name from title | ||
| function getClusterName(title) { | ||
| const prefixes = [ | ||
| "TT0", "TT1", "TT2", "TT3", | ||
| "S1", "M1", "M2", "M4", "W1", | ||
| "Demo 2", "Demo 3", | ||
| "Spec:", "Draft", "Tooling:", "Backlog:" | ||
| ]; | ||
| for (const p of prefixes) { | ||
| if (title.startsWith(p)) return p.replace(":", ""); | ||
| } | ||
| return "Misc"; | ||
| } | ||
|
|
||
| function generateDot(nodes, edges) { | ||
| // Filter out isolated nodes | ||
| const connectedNodeIds = new Set(); | ||
| for (const e of edges) { | ||
| connectedNodeIds.add(e.from); | ||
| connectedNodeIds.add(e.to); | ||
| } | ||
|
|
||
| // Create a filtered map of nodes | ||
| const filteredNodes = new Map(); | ||
| for (const [id, node] of nodes) { | ||
| if (connectedNodeIds.has(id)) { | ||
| filteredNodes.set(id, node); | ||
| } | ||
| } | ||
|
|
||
| const lines = []; | ||
| lines.push('digraph tasks_dag {'); | ||
| lines.push(' graph [rankdir=LR, labelloc="t", fontsize=18, fontname="Helvetica", newrank=true, splines=true];'); | ||
| lines.push(' node [shape=box, style="rounded,filled", fontname="Helvetica", fontsize=10, margin="0.10,0.06"];'); | ||
| lines.push(' edge [fontname="Helvetica", fontsize=9, arrowsize=0.8];'); | ||
| lines.push(' label="Echo — Tasks DAG (from TASKS-DAG.md)\nGenerated by scripts/generate-tasks-dag.js";'); | ||
| lines.push(''); | ||
|
|
||
| // Clusters | ||
| const clusters = new Map(); | ||
| for (const node of filteredNodes.values()) { | ||
| const cluster = getClusterName(node.title); | ||
| if (!clusters.has(cluster)) clusters.set(cluster, []); | ||
| clusters.get(cluster).push(node); | ||
| } | ||
|
|
||
| for (const [name, groupNodes] of clusters) { | ||
| // Sanitize cluster name for ID | ||
| const clusterId = "cluster_" + name.replace(/[^a-zA-Z0-9]/g, "_"); | ||
| lines.push(` subgraph ${clusterId} {`); | ||
| lines.push(` label="${escapeDotString(name)}";`); | ||
| lines.push(' style="rounded"; color="gray70";'); | ||
| // Simple color cycle for clusters | ||
| const colors = ["#dbeafe", "#dcfce7", "#ffedd5", "#f3f4f6", "#fef9c3", "#ede9fe", "#ccfbf1", "#fee2e2"]; | ||
| const color = colors[Math.abs(name.split('').reduce((a,c)=>a+c.charCodeAt(0),0)) % colors.length]; | ||
| lines.push(` node [fillcolor="${color}"];`); | ||
|
|
||
| for (const node of groupNodes) { | ||
| const label = `#${node.number}\n${node.title.replace(/"/g, "'")}`; // escape quotes in label for DOT safety (though escapeDotString handles the attribute wrapper) | ||
| // limit label length? | ||
| let safeLabel = escapeDotString(label); | ||
| if (safeLabel.length > 50) { | ||
| // insert line break roughly | ||
| safeLabel = safeLabel.replace(/(.{30,}?)\s/, "$1\\n"); | ||
| } | ||
|
|
||
| lines.push(` i${node.number} [label="${safeLabel}", URL="${node.url}", tooltip="${escapeDotString(node.title)}"];`); | ||
| } | ||
| lines.push(' }'); | ||
| } | ||
|
|
||
| lines.push(''); | ||
| for (const edge of edges) { | ||
| // Only add edge if both nodes exist in our set (which they should) | ||
| if (nodes.has(edge.from) && nodes.has(edge.to)) { | ||
| lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`); | ||
| } | ||
| } | ||
|
|
||
| lines.push('}'); | ||
| return lines.join("\n"); | ||
| } | ||
|
|
||
| function main() { | ||
| if (!fs.existsSync(INPUT_FILE)) fail(`Input file not found: ${INPUT_FILE}`); | ||
|
|
||
| const content = fs.readFileSync(INPUT_FILE, "utf8"); | ||
| const { nodes, edges } = parseTasksDag(content); | ||
|
|
||
| const dotContent = generateDot(nodes, edges); | ||
|
|
||
| if (!fs.existsSync(OUT_DIR)) fs.mkdirSync(OUT_DIR, { recursive: true }); | ||
| fs.writeFileSync(DOT_FILE, dotContent); | ||
| console.log(`Wrote DOT file to ${DOT_FILE}`); | ||
|
|
||
| try { | ||
| runChecked("dot", ["-Tsvg", DOT_FILE, "-o", SVG_FILE]); | ||
| console.log(`Rendered SVG to ${SVG_FILE}`); | ||
| } catch (e) { | ||
| console.warn("Warning: Failed to render SVG (is graphviz installed?). Only DOT file generated."); | ||
| } | ||
| } | ||
|
|
||
| main(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for TASKS-DAG.md and what processes interact with it
rg -n "TASKS-DAG" --type-list | head
cd /tmp && pwd
find /repo -name "TASKS-DAG.md" -o -name "*TASKS*DAG*" 2>/dev/null | head -20Repository: flyingrobots/echo
Length of output: 472
🏁 Script executed:
# Look for any scripts that write to TASKS-DAG.md
rg "TASKS-DAG\.md" -g "*.js" -g "*.ts" -g "*.py" -g "*.sh" -B 2 -A 2Repository: flyingrobots/echo
Length of output: 858
🏁 Script executed:
# Check GitHub Actions and workflows
find /repo -path "*/.github/workflows" -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -10Repository: flyingrobots/echo
Length of output: 43
🏁 Script executed:
# Check the actual directory structure for dags
fd "dags" --type dRepository: flyingrobots/echo
Length of output: 98
🏁 Script executed:
# Look for documentation about DAGs
fd "dependency-dags" -o -name "*dag*" -path "*/docs/*" --type fRepository: flyingrobots/echo
Length of output: 291
🏁 Script executed:
# List contents of both dags directories
echo "=== Contents of docs/assets/dags/ ==="
ls -la docs/assets/dags/ 2>/dev/null | head -20
echo "=== Contents of docs/assets/dags-2/ ==="
ls -la docs/assets/dags-2/ 2>/dev/null | head -20
echo "=== Check if TASKS-DAG.md exists ==="
ls -la TASKS-DAG.md 2>/dev/nullRepository: flyingrobots/echo
Length of output: 930
🏁 Script executed:
# Search for all workflow files
find . -name "*.yml" -o -name "*.yaml" | grep -E "(workflows|\.github)" | head -20Repository: flyingrobots/echo
Length of output: 479
🏁 Script executed:
# Search for any file that writes to or generates TASKS-DAG
rg "writeFileSync|appendFileSync" -g "*.js" -g "*.ts" -B 2 -A 2 | grep -A 5 -B 5 "TASKS"Repository: flyingrobots/echo
Length of output: 43
🏁 Script executed:
# Check if docs/dependency-dags.md exists and what it contains
cat docs/dependency-dags.md 2>/dev/null || echo "File not found"Repository: flyingrobots/echo
Length of output: 2300
🏁 Script executed:
# Look for any references to dags-2
rg "dags-2" -g "*.md" -g "*.js" -g "*.yml" -B 2 -A 2Repository: flyingrobots/echo
Length of output: 923
🏁 Script executed:
# Check the refresh workflow
cat .github/workflows/refresh-dependency-dags.yml | head -100Repository: flyingrobots/echo
Length of output: 2094
🏁 Script executed:
# Look for any script that might generate TASKS-DAG.md
rg "TASKS-DAG" . -g "*.js" -g "*.ts" -g "*.py" -g "*.sh" -B 5 -A 5Repository: flyingrobots/echo
Length of output: 1663
🏁 Script executed:
# Check if there's a generate-dependency-dags script
ls -la scripts/generate-* 2>/dev/nullRepository: flyingrobots/echo
Length of output: 226
🏁 Script executed:
# Look at the beginning of TASKS-DAG.md
head -50 TASKS-DAG.mdRepository: flyingrobots/echo
Length of output: 2644
🏁 Script executed:
# Verify the learning more thoroughly - check actual docs structure
cat docs/dependency-dags.md | grep -A 5 "Regenerating\|regenerat"Repository: flyingrobots/echo
Length of output: 241
🏁 Script executed:
# Check if there are any TODOs or FIXMEs in the script or docs about dags-2
rg "TODO|FIXME|XXX|BUG" scripts/generate-tasks-dag.js docs/dependency-dags-2.mdRepository: flyingrobots/echo
Length of output: 43
🏁 Script executed:
# Check the Makefile or xtask for dags generation
grep -r "dags\|tasks-dag" Makefile Cargo.toml 2>/dev/null | head -20Repository: flyingrobots/echo
Length of output: 193
🏁 Script executed:
# Check xtask implementation for dags
fd -t f "xtask\|xtask.rs" | xargs grep -l "dags" 2>/dev/nullRepository: flyingrobots/echo
Length of output: 45
🏁 Script executed:
# Verify if TASKS-DAG.md is actually generated anywhere
rg "fs.writeFileSync.*TASKS-DAG" -g "*.js" -g "*.ts" -g "*.rs" -B 5 -A 5Repository: flyingrobots/echo
Length of output: 43
🏁 Script executed:
# Check if there's any git history or comments indicating TASKS-DAG.md is meant to be auto-generated
rg "TASKS-DAG|tasks-dag" docs/dependency-dags-2.md -B 3 -A 3Repository: flyingrobots/echo
Length of output: 986
Move output to docs/assets/dags/ and integrate into refresh workflow.
The script outputs to docs/assets/dags-2/ which violates the established convention documented in learnings: all planning DAG artifacts belong in docs/assets/dags/ and should be consolidated in docs/dependency-dags.md. Additionally, TASKS-DAG.md claims "Auto-generated" but is entirely manually curated with no automation—the script that consumes it is orphaned from the refresh workflow and runs nowhere. The parallel directory structure and fragmented documentation (docs/dependency-dags-2.md) break consistency and discoverability.
Required fixes:
- Change
OUT_DIRfrom"docs/assets/dags-2"to"docs/assets/dags"(renaming outputs totasks-dag.dot/tasks-dag.svgif needed to avoid collisions) - Remove the false "Auto-generated" header from
TASKS-DAG.mdor implement actual automation to regenerate it - Integrate
generate-tasks-dag.jsintorefresh-dependency-dags.ymlworkflow so it runs alongside other DAG refreshes - Consolidate documentation into
docs/dependency-dags.mdinstead of maintaining separatedocs/dependency-dags-2.md
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 1-226, the script currently writes
outputs to docs/assets/dags-2 which breaks project conventions; update OUT_DIR
from "docs/assets/dags-2" to "docs/assets/dags" (ensure DOT/SVG filenames remain
tasks-dag.dot/tasks-dag.svg or rename consistently to avoid collisions), remove
the misleading "Auto-generated" header from TASKS-DAG.md (or replace it by
wiring real generation into the refresh workflow), add this script to the
refresh-dependency-dags.yml workflow so it runs during DAG refreshes, and
consolidate any separate docs (docs/dependency-dags-2.md) into
docs/dependency-dags.md so all DAG artifacts and documentation live under
docs/assets/dags and docs/dependency-dags.md respectively.
| let currentIssue = null; | ||
| let mode = null; // 'blocks' or 'blocked_by' | ||
|
|
||
| const issueRegex = /^## \[#(\d+): (.+)](.+)"/; |
There was a problem hiding this comment.
Dead code: Unused regex with malformed pattern.
Line 32 defines issueRegex with a malformed pattern: /^## \[#(\d+): (.+)](.+)"/ (note the mismatched brackets and quotes). This variable is never used—line 41 defines a corrected inline regex instead.
🔎 Remove dead code or extract working regex
Either remove the unused variable:
- const issueRegex = /^## \[#(\d+): (.+)](.+)"/;
const linkRegex = /^\s+- \[#(\d+): (.*?)\]\((.*)\)/;Or extract the working regex from line 41:
+ const issueRegex = /^## \[#(\d+): (.*?)\]\((.*)\)/;
- const issueRegex = /^## \[#(\d+): (.+)](.+)"/;
const linkRegex = /^\s+- \[#(\d+): (.*?)\]\((.*)\)/;Then use it at line 41:
- const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/);
+ const issueMatch = line.match(issueRegex);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const issueRegex = /^## \[#(\d+): (.+)](.+)"/; | |
| const linkRegex = /^\s+- \[#(\d+): (.*?)\]\((.*)\)/; |
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around line 32, there's an unused, malformed
regex declaration `issueRegex` (/^## \[#(\d+): (.+)](.+)"/) — remove this dead
variable or replace it with the corrected regex used on line 41 (ensure no stray
quote and properly escaped brackets), then reference that variable at line 41
instead of the inline regex so the pattern is defined once and used
consistently.
| const label = `#${node.number}\n${node.title.replace(/"/g, "'")}`; // escape quotes in label for DOT safety (though escapeDotString handles the attribute wrapper) | ||
| // limit label length? | ||
| let safeLabel = escapeDotString(label); | ||
| if (safeLabel.length > 50) { | ||
| // insert line break roughly | ||
| safeLabel = safeLabel.replace(/(.{30,}?)\s/, "$1\\n"); | ||
| } | ||
|
|
||
| lines.push(` i${node.number} [label="${safeLabel}", URL="${node.url}", tooltip="${escapeDotString(node.title)}"];`); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing escaping for URL attribute.
Line 189 directly interpolates node.url without escaping. While GitHub URLs are unlikely to contain problematic characters, defensive programming dictates escaping all string attributes in DOT output.
Additionally, line 181's .replace(/"/g, "'") is redundant since escapeDotString at line 183 already handles quote escaping properly.
🔎 Escape URL and remove redundant replace
- const label = `#${node.number}\n${node.title.replace(/"/g, "'")}`;
+ const label = `#${node.number}\n${node.title}`;
// escape quotes in label for DOT safety (though escapeDotString handles the attribute wrapper)
let safeLabel = escapeDotString(label);
if (safeLabel.length > 50) {
// insert line break roughly
safeLabel = safeLabel.replace(/(.{30,}?)\s/, "$1\\n");
}
- lines.push(` i${node.number} [label="${safeLabel}", URL="${node.url}", tooltip="${escapeDotString(node.title)}"];`);
+ lines.push(` i${node.number} [label="${safeLabel}", URL="${escapeDotString(node.url)}", tooltip="${escapeDotString(node.title)}"];`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const label = `#${node.number}\n${node.title.replace(/"/g, "'")}`; // escape quotes in label for DOT safety (though escapeDotString handles the attribute wrapper) | |
| // limit label length? | |
| let safeLabel = escapeDotString(label); | |
| if (safeLabel.length > 50) { | |
| // insert line break roughly | |
| safeLabel = safeLabel.replace(/(.{30,}?)\s/, "$1\\n"); | |
| } | |
| lines.push(` i${node.number} [label="${safeLabel}", URL="${node.url}", tooltip="${escapeDotString(node.title)}"];`); | |
| const label = `#${node.number}\n${node.title}`; | |
| // escape quotes in label for DOT safety (though escapeDotString handles the attribute wrapper) | |
| let safeLabel = escapeDotString(label); | |
| if (safeLabel.length > 50) { | |
| // insert line break roughly | |
| safeLabel = safeLabel.replace(/(.{30,}?)\s/, "$1\\n"); | |
| } | |
| lines.push(` i${node.number} [label="${safeLabel}", URL="${escapeDotString(node.url)}", tooltip="${escapeDotString(node.title)}"];`); |
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 181 to 189, the code redundantly
replaces double quotes in the title before calling escapeDotString and emits
node.url unescaped into the DOT attribute; remove the .replace(/"/g, "'") step
and instead pass the original title into escapeDotString, and wrap node.url with
escapeDotString when interpolating the URL and tooltip attributes so all DOT
string attributes are properly escaped; keep the safeLabel length-break logic
but apply it to the escaped label value to ensure line breaks are inserted on
the safe string.
| for (const edge of edges) { | ||
| // Only add edge if both nodes exist in our set (which they should) | ||
| if (nodes.has(edge.from) && nodes.has(edge.to)) { | ||
| lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent data structure reference in edge validation.
Line 197 checks nodes.has() but should check filteredNodes.has() for logical consistency. While this doesn't cause a bug in practice (isolated nodes by definition have no edges), the check should reference the data structure that's actually output.
🔎 Use filteredNodes for consistency
for (const edge of edges) {
- // Only add edge if both nodes exist in our set (which they should)
- if (nodes.has(edge.from) && nodes.has(edge.to)) {
+ // Only add edge if both nodes exist in our filtered set
+ if (filteredNodes.has(edge.from) && filteredNodes.has(edge.to)) {
lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const edge of edges) { | |
| // Only add edge if both nodes exist in our set (which they should) | |
| if (nodes.has(edge.from) && nodes.has(edge.to)) { | |
| lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`); | |
| } | |
| } | |
| for (const edge of edges) { | |
| // Only add edge if both nodes exist in our filtered set | |
| if (filteredNodes.has(edge.from) && filteredNodes.has(edge.to)) { | |
| lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 195 to 200, the edge-validation
condition currently uses nodes.has(...) but should use filteredNodes.has(...)
for consistency with the set that is actually output; update the if to check
filteredNodes.has(edge.from) && filteredNodes.has(edge.to) (ensure filteredNodes
is in scope) so edges are validated against the filtered node set rather than
the original nodes set.
| try { | ||
| runChecked("dot", ["-Tsvg", DOT_FILE, "-o", SVG_FILE]); | ||
| console.log(`Rendered SVG to ${SVG_FILE}`); | ||
| } catch (e) { | ||
| console.warn("Warning: Failed to render SVG (is graphviz installed?). Only DOT file generated."); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Error swallowing in SVG rendering.
The catch block at line 221 suppresses the error without logging it. While the warning mentions Graphviz, the failure could be due to other issues (permissions, disk space, etc.). Log the actual error for better debuggability.
🔎 Log the actual error
try {
runChecked("dot", ["-Tsvg", DOT_FILE, "-o", SVG_FILE]);
console.log(`Rendered SVG to ${SVG_FILE}`);
} catch (e) {
- console.warn("Warning: Failed to render SVG (is graphviz installed?). Only DOT file generated.");
+ console.warn(`Warning: Failed to render SVG: ${e.message}. Is graphviz installed? Only DOT file generated.`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| runChecked("dot", ["-Tsvg", DOT_FILE, "-o", SVG_FILE]); | |
| console.log(`Rendered SVG to ${SVG_FILE}`); | |
| } catch (e) { | |
| console.warn("Warning: Failed to render SVG (is graphviz installed?). Only DOT file generated."); | |
| } | |
| try { | |
| runChecked("dot", ["-Tsvg", DOT_FILE, "-o", SVG_FILE]); | |
| console.log(`Rendered SVG to ${SVG_FILE}`); | |
| } catch (e) { | |
| console.warn(`Warning: Failed to render SVG: ${e.message}. Is graphviz installed? Only DOT file generated.`); | |
| } |
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 218 to 223, the catch block for
the SVG rendering swallows the thrown error; update the catch to log the actual
error details (e.g., include the caught exception or its message/stack in the
warning) while preserving the existing user-facing warning about Graphviz so
failures from permissions, disk space, or other causes are visible for
debugging.
Adds the standard confidence legend (strong/medium/weak) to the generated tasks graph so the edge styles are self-explanatory.
Updates dependency dag generators to color-code edges: - Plan (issue-deps): Green if confirmed in Issue Body, Red if missing from Plan. - Reality (tasks-dag): All edges Green (confirmed by definition). This visualization highlights the divergence between the config-based plan and the actual issue-tracker state.
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
docs/assets/dags-2/tasks-dag.dotis excluded by!**/*.dotdocs/assets/dags-2/tasks-dag.svgis excluded by!**/*.svgdocs/assets/dags/issue-deps.dotis excluded by!**/*.dotdocs/assets/dags/issue-deps.svgis excluded by!**/*.svgdocs/assets/dags/milestone-deps.dotis excluded by!**/*.dotdocs/assets/dags/milestone-deps.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
scripts/generate-dependency-dags.jsscripts/generate-tasks-dag.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T02:01:23.896Z
Learning: Applies to docs/assets/dags/** : Planning DAG artifacts live in `docs/assets/dags/` and should be documented in `docs/dependency-dags.md`; set `DAG_REFRESH_ISSUE=<issue-number>` for automated DAG refresh PRs
📚 Learning: 2026-01-03T02:01:23.896Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-03T02:01:23.896Z
Learning: Applies to docs/assets/dags/** : Planning DAG artifacts live in `docs/assets/dags/` and should be documented in `docs/dependency-dags.md`; set `DAG_REFRESH_ISSUE=<issue-number>` for automated DAG refresh PRs
Applied to files:
scripts/generate-dependency-dags.jsscripts/generate-tasks-dag.js
🔇 Additional comments (3)
scripts/generate-dependency-dags.js (3)
179-179: LGTM: cleaner GitHub API query format.The shift from multiple
-fflags to an explicit query string is more readable and equally valid.
336-337: LGTM: clear legend for reality overlay.The new legend entries accurately describe the green (confirmed) and red (missing from plan) edges.
534-538: LGTM: clean optional loading of reality edges.The conditional read of
TASKS-DAG.mdgracefully handles its absence and integrates reality edges when available.
| function parseTasksDag(content) { | ||
| const lines = content.split("\n"); | ||
| const edges = new Set(); // "from->to" strings | ||
|
|
||
| let currentIssue = null; | ||
| let mode = null; // 'blocks' or 'blocked_by' | ||
|
|
||
| for (const line of lines) { | ||
| if (line.startsWith("## [")) { | ||
| const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/); | ||
| if (issueMatch) { | ||
| currentIssue = parseInt(issueMatch[1], 10); | ||
| mode = null; | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (!currentIssue) continue; | ||
|
|
||
| if (line.trim() === "- Blocked by:") { | ||
| mode = "blocked_by"; | ||
| continue; | ||
| } | ||
| if (line.trim() === "- Blocks:") { | ||
| mode = "blocks"; | ||
| continue; | ||
| } | ||
|
|
||
| const linkMatch = line.match(/^\s+- \[#(\d+): (.*?)\]\((.*)\)/); | ||
| if (linkMatch) { | ||
| const targetNumber = parseInt(linkMatch[1], 10); | ||
| if (mode === "blocked_by") { | ||
| // Target -> Current | ||
| edges.add(`${targetNumber}->${currentIssue}`); | ||
| } else if (mode === "blocks") { | ||
| // Current -> Target | ||
| edges.add(`${currentIssue}->${targetNumber}`); | ||
| } | ||
| } | ||
| } | ||
| return edges; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Lack of error handling for malformed edge formats.
The function parses TASKS-DAG.md and returns edge strings like "123->456", but provides no validation or error handling for malformed content. If the Markdown is corrupted, parseInt could return NaN and propagate silently through the edge set. While unlikely in practice, defensive parsing would catch format violations early.
🔎 Add validation
function parseTasksDag(content) {
const lines = content.split("\n");
const edges = new Set(); // "from->to" strings
let currentIssue = null;
let mode = null; // 'blocks' or 'blocked_by'
for (const line of lines) {
if (line.startsWith("## [")) {
const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/);
if (issueMatch) {
currentIssue = parseInt(issueMatch[1], 10);
+ if (isNaN(currentIssue)) {
+ console.warn(`Warning: Invalid issue number in line: ${line}`);
+ currentIssue = null;
+ continue;
+ }
mode = null;
continue;
}
}
if (!currentIssue) continue;
if (line.trim() === "- Blocked by:") {
mode = "blocked_by";
continue;
}
if (line.trim() === "- Blocks:") {
mode = "blocks";
continue;
}
const linkMatch = line.match(/^\s+- \[#(\d+): (.*?)\]\((.*)\)/);
if (linkMatch) {
const targetNumber = parseInt(linkMatch[1], 10);
+ if (isNaN(targetNumber)) {
+ console.warn(`Warning: Invalid target number in line: ${line}`);
+ continue;
+ }
if (mode === "blocked_by") {
// Target -> Current
edges.add(`${targetNumber}->${currentIssue}`);
} else if (mode === "blocks") {
// Current -> Target
edges.add(`${currentIssue}->${targetNumber}`);
}
}
}
return edges;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parseTasksDag(content) { | |
| const lines = content.split("\n"); | |
| const edges = new Set(); // "from->to" strings | |
| let currentIssue = null; | |
| let mode = null; // 'blocks' or 'blocked_by' | |
| for (const line of lines) { | |
| if (line.startsWith("## [")) { | |
| const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/); | |
| if (issueMatch) { | |
| currentIssue = parseInt(issueMatch[1], 10); | |
| mode = null; | |
| continue; | |
| } | |
| } | |
| if (!currentIssue) continue; | |
| if (line.trim() === "- Blocked by:") { | |
| mode = "blocked_by"; | |
| continue; | |
| } | |
| if (line.trim() === "- Blocks:") { | |
| mode = "blocks"; | |
| continue; | |
| } | |
| const linkMatch = line.match(/^\s+- \[#(\d+): (.*?)\]\((.*)\)/); | |
| if (linkMatch) { | |
| const targetNumber = parseInt(linkMatch[1], 10); | |
| if (mode === "blocked_by") { | |
| // Target -> Current | |
| edges.add(`${targetNumber}->${currentIssue}`); | |
| } else if (mode === "blocks") { | |
| // Current -> Target | |
| edges.add(`${currentIssue}->${targetNumber}`); | |
| } | |
| } | |
| } | |
| return edges; | |
| } | |
| function parseTasksDag(content) { | |
| const lines = content.split("\n"); | |
| const edges = new Set(); // "from->to" strings | |
| let currentIssue = null; | |
| let mode = null; // 'blocks' or 'blocked_by' | |
| for (const line of lines) { | |
| if (line.startsWith("## [")) { | |
| const issueMatch = line.match(/^## \[#(\d+): (.*?)\]\((.*)\)/); | |
| if (issueMatch) { | |
| currentIssue = parseInt(issueMatch[1], 10); | |
| if (isNaN(currentIssue)) { | |
| console.warn(`Warning: Invalid issue number in line: ${line}`); | |
| currentIssue = null; | |
| continue; | |
| } | |
| mode = null; | |
| continue; | |
| } | |
| } | |
| if (!currentIssue) continue; | |
| if (line.trim() === "- Blocked by:") { | |
| mode = "blocked_by"; | |
| continue; | |
| } | |
| if (line.trim() === "- Blocks:") { | |
| mode = "blocks"; | |
| continue; | |
| } | |
| const linkMatch = line.match(/^\s+- \[#(\d+): (.*?)\]\((.*)\)/); | |
| if (linkMatch) { | |
| const targetNumber = parseInt(linkMatch[1], 10); | |
| if (isNaN(targetNumber)) { | |
| console.warn(`Warning: Invalid target number in line: ${line}`); | |
| continue; | |
| } | |
| if (mode === "blocked_by") { | |
| // Target -> Current | |
| edges.add(`${targetNumber}->${currentIssue}`); | |
| } else if (mode === "blocks") { | |
| // Current -> Target | |
| edges.add(`${currentIssue}->${targetNumber}`); | |
| } | |
| } | |
| } | |
| return edges; | |
| } |
🤖 Prompt for AI Agents
In scripts/generate-dependency-dags.js around lines 217 to 258, the parser
currently uses parseInt on matched issue and target IDs without validating
results; update the function to validate that both currentIssue and targetNumber
are finite integers (not NaN) before adding edges, and when a malformed ID is
encountered either throw a descriptive Error (including the offending line
content and line number) to fail fast or at minimum log a warning and skip that
line; ensure edges are only added for validated integers and adjust control flow
so mode and currentIssue handling remains correct when skipping invalid entries.
| // Add nodes from reality edges if they exist in the issue snapshot | ||
| if (realityEdges) { | ||
| for (const edgeKey of realityEdges) { | ||
| const [u, v] = edgeKey.split("->").map(n => parseInt(n, 10)); | ||
| // Only add to graph if both nodes are in the issue snapshot (sanity check) | ||
| if (byNum.has(u) && byNum.has(v)) { | ||
| // We generally only add nodes if they are connected to the "Plan" or extend it. | ||
| // For visual clarity, let's include them if they touch the existing Plan nodes or imply missing plan parts. | ||
| // For now, let's purely strictly add them if they are part of a Red edge. | ||
| if (!configuredEdges.has(edgeKey)) { | ||
| nodes.add(u); | ||
| nodes.add(v); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
No error handling for malformed reality edge keys.
Line 276 assumes every edge key from realityEdges follows the "123->456" format. If TASKS-DAG.md is malformed or parseTasksDag produces invalid keys, the split("->") will silently create garbage or throw. Add validation to catch format errors early.
Additionally, the comment at line 281 is confusing: it says "let's purely strictly add them if they are part of a Red edge" but the condition !configuredEdges.has(edgeKey) already ensures we're only adding nodes for missing (red) edges. The comment reads as if we're still deciding, but the condition has already decided.
🔎 Add validation and clarify comment
// Add nodes from reality edges if they exist in the issue snapshot
if (realityEdges) {
for (const edgeKey of realityEdges) {
- const [u, v] = edgeKey.split("->").map(n => parseInt(n, 10));
+ const parts = edgeKey.split("->");
+ if (parts.length !== 2) {
+ console.warn(`Warning: Malformed reality edge key: ${edgeKey}`);
+ continue;
+ }
+ const u = parseInt(parts[0], 10);
+ const v = parseInt(parts[1], 10);
+ if (isNaN(u) || isNaN(v)) {
+ console.warn(`Warning: Invalid issue numbers in reality edge: ${edgeKey}`);
+ continue;
+ }
// Only add to graph if both nodes are in the issue snapshot (sanity check)
if (byNum.has(u) && byNum.has(v)) {
- // We generally only add nodes if they are connected to the "Plan" or extend it.
- // For visual clarity, let's include them if they touch the existing Plan nodes or imply missing plan parts.
- // For now, let's purely strictly add them if they are part of a Red edge.
+ // Add nodes only for red edges (reality edges missing from plan)
if (!configuredEdges.has(edgeKey)) {
nodes.add(u);
nodes.add(v);
}
}
}
}🤖 Prompt for AI Agents
In scripts/generate-dependency-dags.js around lines 273 to 288, edgeKey handling
assumes a strict "123->456" format which can break on malformed input; validate
each edgeKey before using it by either testing against a regex like
/^\s*\d+\s*->\s*\d+\s*$/ or by splitting and checking parts.length === 2 and
that both parseInt results are not NaN (use trim() before parseInt); on invalid
keys skip the entry and optionally log a warning to aid debugging. Also
simplify/clarify the comment: replace the ambiguous "for now..." sentence with a
single line noting that the block only adds nodes for reality edges that are
missing from configuredEdges (i.e., red/missing edges), since the
!configuredEdges.has(edgeKey) check already enforces that.
| const missing = [...nodes].filter((n) => !byNum.has(n)).sort((a, b) => a - b); | ||
| // Warning only for missing nodes in reality edges (dynamic), strict fail for config edges | ||
| // actually existing logic fails on missing config nodes. Let's keep that. | ||
|
|
||
| // Filter nodes that don't exist in byNum (stale config or stale reality) | ||
| const validNodes = [...nodes].filter(n => byNum.has(n)); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Misleading comment about missing node handling.
The comment at lines 291-293 claims "Warning only for missing nodes in reality edges (dynamic), strict fail for config edges" but the code doesn't distinguish between the two sources—it simply filters out all missing nodes at line 295. If the intent is to warn about reality-edge mismatches but fail on config mismatches, the logic doesn't implement that. If the comment is outdated and the current behavior (filter everything) is correct, remove the misleading comment.
🔎 Clarify or remove misleading comment
const missing = [...nodes].filter((n) => !byNum.has(n)).sort((a, b) => a - b);
- // Warning only for missing nodes in reality edges (dynamic), strict fail for config edges
- // actually existing logic fails on missing config nodes. Let's keep that.
-
- // Filter nodes that don't exist in byNum (stale config or stale reality)
+ // Filter out stale node references (both from config edges and reality edges)
const validNodes = [...nodes].filter(n => byNum.has(n));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const missing = [...nodes].filter((n) => !byNum.has(n)).sort((a, b) => a - b); | |
| // Warning only for missing nodes in reality edges (dynamic), strict fail for config edges | |
| // actually existing logic fails on missing config nodes. Let's keep that. | |
| // Filter nodes that don't exist in byNum (stale config or stale reality) | |
| const validNodes = [...nodes].filter(n => byNum.has(n)); | |
| const missing = [...nodes].filter((n) => !byNum.has(n)).sort((a, b) => a - b); | |
| // Filter out stale node references (both from config edges and reality edges) | |
| const validNodes = [...nodes].filter(n => byNum.has(n)); |
🤖 Prompt for AI Agents
In scripts/generate-dependency-dags.js around lines 290 to 295, the comment
falsely states that only reality-edge missing nodes are warned and config-edge
missing nodes fail, but the code simply filters out all missing nodes; either
update the code to enforce warnings for reality edges and strict failure for
config edges (by tracking edge source and splitting missing nodes into
reality-vs-config sets and logging or throwing accordingly) or, if current
behavior is intended, replace the misleading comment with a short accurate note
stating that all missing nodes are filtered/ignored by design.
| for (const { from, to, confidence, note } of issueEdges) { | ||
| const edgeKey = `${from}->${to}`; | ||
| const inReality = realityEdges && realityEdges.has(edgeKey); | ||
| const colorAttr = inReality ? 'color="green", penwidth=2.0' : confidenceEdgeAttrs(confidence); | ||
|
|
||
| // If in reality, override style to solid green, preserving existing penwidth boost | ||
| // Actually confidenceEdgeAttrs returns full string. Let's parse or just conditionally use strings. | ||
| let attrs = confidenceEdgeAttrs(confidence); | ||
| if (inReality) { | ||
| // Replace color and penwidth | ||
| attrs = 'color="green3", penwidth=2.0, style="solid"'; | ||
| } | ||
|
|
||
| lines.push( | ||
| ` i${from} -> i${to} [${confidenceEdgeAttrs( | ||
| confidence, | ||
| )}, tooltip="${escapeDotString(note)}"];`, | ||
| ` i${from} -> i${to} [${attrs}, tooltip="${escapeDotString(note)}"];`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inefficient conditional: calls confidenceEdgeAttrs then discards result.
Lines 384-388 unconditionally call confidenceEdgeAttrs(confidence), then immediately replace the result if inReality is true. This wastes the initial function call and creates a dead variable assignment.
🔎 Use ternary for efficiency
for (const { from, to, confidence, note } of issueEdges) {
const edgeKey = `${from}->${to}`;
const inReality = realityEdges && realityEdges.has(edgeKey);
- const colorAttr = inReality ? 'color="green", penwidth=2.0' : confidenceEdgeAttrs(confidence);
-
- // If in reality, override style to solid green, preserving existing penwidth boost
- // Actually confidenceEdgeAttrs returns full string. Let's parse or just conditionally use strings.
- let attrs = confidenceEdgeAttrs(confidence);
- if (inReality) {
- // Replace color and penwidth
- attrs = 'color="green3", penwidth=2.0, style="solid"';
- }
-
+ const attrs = inReality
+ ? 'color="green3", penwidth=2.0, style="solid"'
+ : confidenceEdgeAttrs(confidence);
lines.push(
` i${from} -> i${to} [${attrs}, tooltip="${escapeDotString(note)}"];`,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const { from, to, confidence, note } of issueEdges) { | |
| const edgeKey = `${from}->${to}`; | |
| const inReality = realityEdges && realityEdges.has(edgeKey); | |
| const colorAttr = inReality ? 'color="green", penwidth=2.0' : confidenceEdgeAttrs(confidence); | |
| // If in reality, override style to solid green, preserving existing penwidth boost | |
| // Actually confidenceEdgeAttrs returns full string. Let's parse or just conditionally use strings. | |
| let attrs = confidenceEdgeAttrs(confidence); | |
| if (inReality) { | |
| // Replace color and penwidth | |
| attrs = 'color="green3", penwidth=2.0, style="solid"'; | |
| } | |
| lines.push( | |
| ` i${from} -> i${to} [${confidenceEdgeAttrs( | |
| confidence, | |
| )}, tooltip="${escapeDotString(note)}"];`, | |
| ` i${from} -> i${to} [${attrs}, tooltip="${escapeDotString(note)}"];`, | |
| ); | |
| } | |
| for (const { from, to, confidence, note } of issueEdges) { | |
| const edgeKey = `${from}->${to}`; | |
| const inReality = realityEdges && realityEdges.has(edgeKey); | |
| const attrs = inReality | |
| ? 'color="green3", penwidth=2.0, style="solid"' | |
| : confidenceEdgeAttrs(confidence); | |
| lines.push( | |
| ` i${from} -> i${to} [${attrs}, tooltip="${escapeDotString(note)}"];`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In scripts/generate-dependency-dags.js around lines 377 to 393, the code calls
confidenceEdgeAttrs(confidence) unconditionally and then overwrites its result
when inReality is true, which wastes the call and leaves an unused colorAttr
variable; fix by computing attrs with a single conditional (e.g., use a ternary
or if/else) so confidenceEdgeAttrs is only invoked when inReality is false,
remove the redundant colorAttr variable, and ensure the chosen attrs string is
used in the pushed line.
| // Red edges (Reality - Plan) | ||
| if (realityEdges) { | ||
| for (const edgeKey of realityEdges) { | ||
| if (!configuredEdges.has(edgeKey)) { | ||
| const [u, v] = edgeKey.split("->").map(n => parseInt(n, 10)); | ||
| if (byNum.has(u) && byNum.has(v)) { | ||
| lines.push( | ||
| ` i${u} -> i${v} [color="red", penwidth=2.0, style="dashed", tooltip="Inferred from Issue Body (missing from Plan)"];` | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate lack of error handling for edge key splitting.
Line 399 repeats the same unsafe split("->").map() pattern as line 276. If an edge key is malformed, this will produce invalid node IDs. Consider extracting a helper function to validate and parse edge keys consistently:
🔎 Extract validation helper
Add this helper near the top of the file:
function parseEdgeKey(edgeKey) {
const parts = edgeKey.split("->");
if (parts.length !== 2) {
throw new Error(`Malformed edge key: ${edgeKey}`);
}
const from = parseInt(parts[0], 10);
const to = parseInt(parts[1], 10);
if (isNaN(from) || isNaN(to)) {
throw new Error(`Invalid issue numbers in edge: ${edgeKey}`);
}
return { from, to };
}Then use it:
// Red edges (Reality - Plan)
if (realityEdges) {
for (const edgeKey of realityEdges) {
if (!configuredEdges.has(edgeKey)) {
- const [u, v] = edgeKey.split("->").map(n => parseInt(n, 10));
- if (byNum.has(u) && byNum.has(v)) {
+ try {
+ const { from: u, to: v } = parseEdgeKey(edgeKey);
+ if (byNum.has(u) && byNum.has(v)) {
lines.push(
` i${u} -> i${v} [color="red", penwidth=2.0, style="dashed", tooltip="Inferred from Issue Body (missing from Plan)"];`
);
+ }
+ } catch (e) {
+ console.warn(`Warning: ${e.message}`);
- }
+ }
}
}
}Apply the same helper to line 276.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/generate-dependency-dags.js around lines 395 to 407, the code
unsafely splits edge keys with `split("->").map()` (duplicated from line ~276)
which can produce invalid node IDs for malformed keys; add a single reusable
parseEdgeKey helper near the top of the file that validates the edgeKey (ensure
it splits into exactly two parts, parseInt both parts and verify they are
numbers), then replace the inline split/map calls on lines 276 and 399 with
calls to this helper; on invalid input either throw a clear Error with the
malformed key or skip the edge with a logged warning so downstream code never
receives NaN node IDs.
| if (pendingEdge) { | ||
| const confMatch = line.match(confidenceRegex); | ||
| if (confMatch) { | ||
| pendingEdge.confidence = confMatch[1].trim().toLowerCase(); | ||
| continue; | ||
| } | ||
| const evMatch = line.match(evidenceRegex); | ||
| if (evMatch) { | ||
| pendingEdge.note = evMatch[1].trim(); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Dead code: confidence metadata parsed but never used.
Lines 92-102 meticulously parse confidence and evidence from TASKS-DAG.md, storing them in pendingEdge.confidence and pendingEdge.note. But line 207-208 hardcodes ALL edges to color="green3", penwidth=2.0, style="solid", completely ignoring the parsed confidence. This makes the confidence parsing worthless and prevents the graph from conveying the intended strong/medium/weak distinctions.
🔎 Two options to fix
Option 1: Remove dead confidence parsing
If all edges should be green, remove the unused parsing:
// Metadata for the pending edge
if (pendingEdge) {
- const confMatch = line.match(confidenceRegex);
- if (confMatch) {
- pendingEdge.confidence = confMatch[1].trim().toLowerCase();
- continue;
- }
const evMatch = line.match(evidenceRegex);
if (evMatch) {
pendingEdge.note = evMatch[1].trim();
continue;
}
}Option 2: Actually use the parsed confidence (recommended)
for (const edge of edges) {
// Only add edge if both nodes exist in our set (which they should)
if (nodes.has(edge.from) && nodes.has(edge.to)) {
- // Force Green for "Confirmed in Issue Body" (which is everything here)
- lines.push(` i${edge.from} -> i${edge.to} [color="green3", penwidth=2.0, style="solid", tooltip="${escapeDotString(edge.note || "")}"];`);
+ lines.push(` i${edge.from} -> i${edge.to} [${confidenceAttrs(edge.confidence)}, tooltip="${escapeDotString(edge.note || "")}"];`);
}
}Also applies to: 207-208
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 92-102 and 207-208, the parser
records pendingEdge.confidence but the generated DOT lines later hardcode all
edges to green/solid, rendering the confidence parsing dead; either remove the
parsing or (recommended) map parsed confidence to visual attributes: read
pendingEdge.confidence values ("strong"/"medium"/"weak" or similar), set edge
attributes accordingly (e.g., strong -> color="green3", penwidth=2.5,
style="solid"; medium -> color="orange", penwidth=2.0, style="solid"; weak ->
color="red", penwidth=1.0, style="dashed"), fall back to defaults if missing,
and update the code that emits edges at lines ~207-208 to use these computed
attributes instead of the hardcoded ones.
| lines.push(" subgraph cluster_legend {"); | ||
| lines.push(' label="Legend";'); | ||
| lines.push(' color="gray70";'); | ||
| lines.push(' fontcolor="gray30";'); | ||
| lines.push(' style="rounded";'); | ||
| lines.push(' LG [label="confirmed in issue body", color="green", fontcolor="green"];'); | ||
| lines.push(" }"); | ||
| lines.push(""); |
There was a problem hiding this comment.
Misleading legend: claims "confirmed in issue body" but script reads TASKS-DAG.md.
The legend at line 166 states "confirmed in issue body", but this script parses TASKS-DAG.md (line 8), not issue bodies. This label is copy-pasted from generate-dependency-dags.js where it's accurate, but here it's completely wrong and will confuse anyone reading the generated graph.
🔎 Fix the legend text
- LG [label="confirmed in issue body", color="green", fontcolor="green"];
+ LG [label="declared in TASKS-DAG.md", color="green", fontcolor="green"];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lines.push(" subgraph cluster_legend {"); | |
| lines.push(' label="Legend";'); | |
| lines.push(' color="gray70";'); | |
| lines.push(' fontcolor="gray30";'); | |
| lines.push(' style="rounded";'); | |
| lines.push(' LG [label="confirmed in issue body", color="green", fontcolor="green"];'); | |
| lines.push(" }"); | |
| lines.push(""); | |
| lines.push(" subgraph cluster_legend {"); | |
| lines.push(' label="Legend";'); | |
| lines.push(' color="gray70";'); | |
| lines.push(' fontcolor="gray30";'); | |
| lines.push(' style="rounded";'); | |
| lines.push(' LG [label="declared in TASKS-DAG.md", color="green", fontcolor="green"];'); | |
| lines.push(" }"); | |
| lines.push(""); |
🤖 Prompt for AI Agents
In scripts/generate-tasks-dag.js around lines 161 to 168, the legend label
"confirmed in issue body" is incorrect because this script parses TASKS-DAG.md
(see line 8); update the legend text to accurately reflect the source (e.g.,
"confirmed in TASKS-DAG.md" or "confirmed in tasks DAG file") by replacing that
label string (LG [label=...]) with the corrected wording so the generated graph
is not misleading.
Configures AGENTS.md, docs/decision-log.md, docs/execution-plan.md, and TASKS-DAG.md to use the 'union' merge driver. This automatically resolves conflicts by accepting lines from both sides, which is appropriate for these append-only log files.