feat(issues): add --graph flag to issue view for transitive dependency graph - #273
Conversation
…y graph
Adds `lc issue view ISSUE --graph` which traverses blocks relations in
both directions via BFS and renders an ASCII dependency diagram, an
Issues table with status and (root) marker, and an Edges table.
`--output json` emits `{ root, nodes, edges }` instead.
`--graph --web` together fail with a usage error (exit 22).
IssueRelation.endpoint_fields now includes `state { name }` so graph
nodes carry workflow status. The BFS visitor uses a MapSet to handle
cycles and shared dependencies safely; partial API failures abort with
the failing issue identifier.
Closes EXT-56
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
An unresolved critical test compilation failure and multiple moderate graph correctness issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds lc issue view ISSUE --graph for transitive blocks dependency graphs, with BFS traversal, text/JSON rendering, CLI validation, and status-aware relation data.
Changes:
- Adds cycle-safe graph traversal with filtering, sorting, and node limits.
- Adds ASCII/table and JSON graph output.
- Wires the CLI flag, endpoint status fields, documentation, and tests.
File summaries
| File | Summary and review notes |
|---|---|
documents/ash-domain-erd.adoc |
Documents graph and relation changes. No findings. |
app/test/linear_cli/cli/commands/issues/graph_test.exs |
Adds graph coverage. Critical (1 vote): unused import triggers a warnings-as-errors compilation failure. |
app/lib/linear_cli/linear/issue_relation.ex |
Adds endpoint workflow-state fields. No findings. |
app/lib/linear_cli/cli/display.ex |
Renders graph output. Moderate (1 vote): ASCII diagrams omit deeper and predecessor paths. Nit (3 votes): documentation references nonexistent Graph.build/1 instead of the two-argument function. |
app/lib/linear_cli/cli/commands/issues/read.ex |
Integrates graph mode. Moderate (2 votes): UUID roots can be duplicated and reported incorrectly. Moderate (1 vote): root relations are fetched redundantly, risking inconsistent snapshots. |
app/lib/linear_cli/cli/commands/issues/graph.ex |
Implements BFS traversal. Moderate (3 votes): endpoint batches can exceed the advertised 100-node cap; admission and edges must be truncated consistently. |
app/lib/linear_cli/cli.ex |
Registers the --graph flag. No findings. |
Review details
Suppressed comments (3)
app/lib/linear_cli/cli/commands/issues/graph.ex:81
- When
Linear.issue_relations/1fails, this returns{:error, {id, reason}}, whichissue_view/2passes directly intoCLI.run/3.handle_error/3has no clause for that nested tuple, so the command bypasses the user-facing{:smells_bad, message}path instead of reporting the failing issue identifier clearly (and the generic formatter may receive a non-exception). Convert this failure to a handled CLI error or add a dedicated handler for the graph error shape.
case Linear.issue_relations(id) do
{:error, reason} ->
{:error, {id, reason}}
app/lib/linear_cli/cli/commands/issues/read.ex:87
- Graph mode first calls
Linear.issues/1, whoseIssue.full_fields/0already selects both relation connections (app/lib/linear_cli/linear/issue.ex:99-111), and thenGraph.build/2fetches both connections for the root again. This adds redundant API calls and can observe an inconsistent root snapshot; use a root fetch without relations or reuse the preloaded, paginated relation data.
with {:ok, [issue]} <- Linear.issues(%{ids: [expanded_id]}),
{:ok, graph} <- Graph.build(expanded_id, issue) do
app/lib/linear_cli/cli/display.ex:282
- This builds the diagram from only the root's direct successors and one additional successor level. A transitive chain such as root -> B -> C -> D therefore renders only root -> B -> C, while the tables contain D; predecessor chains and deeper branches are omitted, so the ASCII diagram does not represent the graph that was traversed.
# For each successor, gather their successors for chaining
succ_succ_map =
Map.new(succ_ids, fn id ->
ids = Map.get(out_adj, id, [])
- Files reviewed: 7/7 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Replace case/if-else blocks with function clause dispatch and with/else in Graph module (bougyman review) - Extract chain_succ/3 and succ_arrow/2 helpers to replace case in Display.show_graph rendering (bougyman review) - Split show_graph/2 into pattern-matched function heads to eliminate if-else (bougyman review) - Fix Graph.build identifier: use issue.identifier (not expanded_id) as graph root key to avoid UUID/identifier duplication (Copilot review) - Enforce @max_nodes cap in add_endpoint guard clause; only add edges when both endpoints are in the node map (Copilot review) - Fix docstring arity: Graph.build/1 -> Graph.build/2 (Copilot review) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rework summaryAddresses all review feedback from bougyman and Copilot. bougyman style fixesAll
Copilot functional fixes
518 tests pass, credo clean, format clean. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved error handling and incomplete diagram rendering affect correctness; status coverage and module documentation also need updates.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
app/lib/linear_cli/cli/display.ex:285
- This map only gathers one outbound hop for each direct successor of the root. For a transitive chain such as root → A → B → C, the tables contain C but the diagram stops at B; predecessor chains are omitted entirely as well. Render all reachable edges (or explicitly indicate truncation) so the ASCII diagram matches the transitive graph.
app/lib/linear_cli/cli/commands/issues/graph.ex:85
- This error shape is forwarded unchanged by
issue_view/2, but the CLI dispatcher only handles{:error, {:smells_bad, message}}and Ash error structs. A relation-fetch failure therefore falls into the generic WTH handler (exit 88, and potentially an exception while formatting the tuple) instead of reporting the failing issue identifier as promised. Normalize this result in the command path or add a dedicated dispatcher clause.
defp traverse({:error, reason}, id, _rest, _visited, _nodes_map, _edges),
do: {:error, {id, reason}}
app/lib/linear_cli/cli/commands/issues/graph.ex:1
Readnow depends on this newGraphmodule, so the repository's maintenance convention requires adding it to the app module structure inAGENTS.md(AGENTS.md:127-132). The PR updates the domain ERD but leaves the structural module index withoutCLI.Commands.Issues.Graph; add the new issue-command entry there.
defmodule LinearCli.CLI.Commands.Issues.Graph do
app/lib/linear_cli/linear/issue_relation.ex:74
- The new state decoding is what supplies workflow statuses for discovered graph nodes, but the existing IssueRelation fixtures omit
state, and the graph tests never assert a related node's status. This allows the new field to be ignored or decoded incorrectly while all current tests still pass; add afrom_map/2state assertion and an end-to-end graph status assertion.
state = map["state"] && %{name: map["state"]["name"]}
%{
id: map["id"],
identifier: map["identifier"],
title: map["title"],
url: map["url"],
state: state
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
Graph.build/2 returns {:error, {issue_id, reason}} when a relation fetch
fails mid-traversal. The with in read.ex forwards this shape to handle_error/3,
which had no clause for it and fell through to the WTH/exit-88 catch-all.
Add a clause guarded on is_binary(issue_id) that prints the failing issue ID
and re-dispatches the inner reason to the existing handlers.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Run 3 — Copilot follow-up: handle graph relation-fetch error in CLI dispatcherIssue addressed (0e6ca68) Copilot flagged that Fix Added a new defp handle_error({issue_id, reason}, debug, halt) when is_binary(issue_id) do
IO.puts(:stderr, "could not fetch relations for #{issue_id}:")
handle_error(reason, debug, halt)
endThe All 518 tests pass. |
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate issues block approval, with one remaining test-coverage nit.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
app/lib/linear_cli/cli/display.ex:284
- This diagram only collects one successor hop (
out_adjfor each direct successor), and therender_columns/5branch used when the root has predecessors ignores that map entirely. A valid transitive chain such asA -> root -> B -> Ctherefore shows only throughBin the ASCII diagram even thoughCis present in the tables; predecessor chains are similarly truncated. Please render all reachable edges (or explicitly mark the diagram as truncated) so--graphdoes not present an incomplete dependency graph.
app/lib/linear_cli/cli/commands/issues/read.ex:87
- Graph mode first calls
Linear.issues/1, whoseIssue.full_fields/0includes both root relation connections as well as comments, labels, and other full-view data, and thenGraph.build/2fetchesLinear.issue_relations/1for the same root. Every graph invocation therefore repeats the root relation requests and loads a large payload it does not render, adding avoidable latency and API usage; use a graph-specific lightweight issue fetch or reuse/paginate the preloaded root relations.
with {:ok, [issue]} <- Linear.issues(%{ids: [expanded_id]}),
{:ok, graph} <- Graph.build(issue.identifier, issue) do
app/lib/linear_cli/linear/issue_relation.ex:67
state { name }is the new data path needed for graph node status, but the updated relation fixtures omitstate, and the graph tests assert statuses only for the root node. A regression in this decoder would therefore leave the current suite green while all discovered nodes render with empty status; add a relation fixture with endpoint state and assert the discovered node's status.
state = map["state"] && %{name: map["state"]["name"]}
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
…gram Three rendering paths had off-by-one or off-by-two misalignment: - chain_succ/3: pad was root+succ+9, must be root+succ+8 - render_columns/5 no-pred multi-succ: pad was root+1, must be root-1 (so the continuation + lands at root+3, matching the first line) - render_columns/5 preds+multi-succ: succ_arrow embedded in a pred line lost the predecessor column offset entirely; replaced with mid_pred_line/4 dispatch that computes cont_pad = pred_max + root_len + 11 Also adds CLI.Commands.Issues.Graph to AGENTS.md module index (maintenance convention) and adds a discovered-node status assertion to the graph test.
Rework Summary (Run 4)Addressed all three minor items flagged in the code review: 1. ASCII Three rendering paths had off-by-one or off-by-two misalignment in junction connectors:
All formulas verified arithmetically and by rendering concrete examples. 2. AGENTS.md module structure updated Added 3. Discovered node status assertion added to graph tests Added Quality gates: 518 tests pass, credo clean, pre-push hooks pass. |
Summary
lc issue view ISSUE --graphwhich traversesblocksrelations transitively in both directions via BFS, rendering an ASCII dependency diagram, an Issues table (ISSUE / STATUS / TITLE, with(root)marker), and an Edges table with the legendA -> B means A blocks B--output jsonemits{ root, nodes, edges }only — no diagram--graph --webtogether fail immediately with exit 22IssueRelation.endpoint_fieldsnow includesstate { name }so graph nodes carry workflow status (additive, backward-compatible)Test plan
--graphaccepted,--graph --webrejected with exit 22){ root, nodes, edges }with correct keys--output jsonwithout--graphunchanged (issue JSON)issue viewtests unaffected (518 passed, 0 failures)Closes EXT-56
🤖 Generated with Claude Code