Skip to content

feat(issues): add --graph flag to issue view for transitive dependency graph - #273

Merged
ruby-automation merged 4 commits into
mainfrom
EXT-56-graph-dependency
Sep 13, 2026
Merged

feat(issues): add --graph flag to issue view for transitive dependency graph#273
ruby-automation merged 4 commits into
mainfrom
EXT-56-graph-dependency

Conversation

@ruby-automation

Copy link
Copy Markdown
Contributor

Summary

  • Adds lc issue view ISSUE --graph which traverses blocks relations 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 legend A -> B means A blocks B
  • --output json emits { root, nodes, edges } only — no diagram
  • --graph --web together fail immediately with exit 22
  • IssueRelation.endpoint_fields now includes state { name } so graph nodes carry workflow status (additive, backward-compatible)
  • BFS visitor uses a MapSet for cycle-safety and a 100-node cap; partial API failures abort with the failing issue identifier

Test plan

  • Parser wiring (--graph accepted, --graph --web rejected with exit 22)
  • No-deps graph: root node present, no edges
  • Non-blocks relations excluded (related, duplicate)
  • Single outbound and single inbound blocks edges
  • Transitive chain (A → B → C)
  • Branching / shared dependencies
  • Cycle detection terminates cleanly
  • Nodes sorted by identifier; edges sorted by (source, target)
  • Text output: diagram + Issues table + Edges table
  • JSON output shape: { root, nodes, edges } with correct keys
  • --output json without --graph unchanged (issue JSON)
  • Existing issue view tests unaffected (518 passed, 0 failures)
  • pre-commit and pre-push hooks pass (format, credo --strict, full test suite)

Closes EXT-56

🤖 Generated with Claude Code

…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>
Comment thread app/lib/linear_cli/cli/commands/issues/graph.ex Outdated
Comment thread app/lib/linear_cli/cli/commands/issues/graph.ex Outdated
Comment thread app/lib/linear_cli/cli/commands/issues/graph.ex Outdated
Comment thread app/lib/linear_cli/cli/display.ex Outdated
Comment thread app/lib/linear_cli/cli/display.ex Outdated
Comment thread app/lib/linear_cli/cli/display.ex Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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/1 fails, this returns {:error, {id, reason}}, which issue_view/2 passes directly into CLI.run/3. handle_error/3 has 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, whose Issue.full_fields/0 already selects both relation connections (app/lib/linear_cli/linear/issue.ex:99-111), and then Graph.build/2 fetches 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.

Comment thread app/test/linear_cli/cli/commands/issues/graph_test.exs
Comment thread app/lib/linear_cli/cli/commands/issues/graph.ex Outdated
Comment thread app/lib/linear_cli/cli/commands/issues/read.ex Outdated
Comment thread app/lib/linear_cli/cli/display.ex Outdated
- 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>
@ruby-automation

Copy link
Copy Markdown
Contributor Author

Rework summary

Addresses all review feedback from bougyman and Copilot.

bougyman style fixes

All case and multi-line if/else blocks replaced with idiomatic Elixir function clause dispatch:

  • graph.ex: build/2 now uses with instead of case for the BFS result; the visited/cap skip logic uses a visit/6 function with true/false pattern-match clauses; relation-fetch dispatch uses traverse/6 function clauses on {:ok, _}/{:error, _} patterns — no with/else (avoids the one-<--clause credo warning).
  • display.ex: show_graph/2 split into two pattern-matched heads (%{output: "json"} and fallback); render_columns case for single successor chain extracted to chain_succ/3 helper with three clauses; case for predecessor+successor arrow extracted to succ_arrow/2 helper with three clauses.

Copilot functional fixes

  • UUID root deduplication (read.ex): Graph.build/2 now receives issue.identifier (from the fetched struct) instead of expanded_id, so UUIDs passed on the CLI are correctly resolved to the canonical identifier before seeding the graph.
  • @max_nodes cap enforcement (graph.ex): add_endpoint/5 has a guard clause that returns unchanged state when map_size(nodes_map) >= @max_nodes; process_relation/3 now checks Map.has_key?/2 for both source and target before adding an edge, ensuring edges stay consistent with the capped node set.
  • Docstring arity (display.ex): Graph.build/1Graph.build/2.

518 tests pass, credo clean, format clean.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

  • Read now depends on this new Graph module, so the repository's maintenance convention requires adding it to the app module structure in AGENTS.md (AGENTS.md:127-132). The PR updates the domain ERD but leaves the structural module index without CLI.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 a from_map/2 state 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

Comment thread app/lib/linear_cli/cli/commands/issues/read.ex
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>
@ruby-automation

Copy link
Copy Markdown
Contributor Author

Run 3 — Copilot follow-up: handle graph relation-fetch error in CLI dispatcher

Issue addressed (0e6ca68)

Copilot flagged that Graph.build/2 returns {:error, {issue_id, reason}} when a mid-traversal relation fetch fails, but handle_error/3 in cli.ex had no clause for this shape. The inner tuple fell through to the generic WTH/exit-88 catch-all, giving users an opaque crash message instead of a meaningful "could not fetch relations for ISSUE-123" message.

Fix

Added a new handle_error/3 clause before the catch-all:

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)
end

The when is_binary(issue_id) guard keeps this clause scoped to the {issue_id, reason} shape returned by Graph.build/2 without disturbing other tuple-shaped errors. It prints the failing issue ID for context, then re-dispatches the inner reason so it flows through whichever existing handler matches (API errors, auth errors, etc.) rather than duplicating their formatting logic.

All 518 tests pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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_adj for each direct successor), and the render_columns/5 branch used when the root has predecessors ignores that map entirely. A valid transitive chain such as A -> root -> B -> C therefore shows only through B in the ASCII diagram even though C is present in the tables; predecessor chains are similarly truncated. Please render all reachable edges (or explicitly mark the diagram as truncated) so --graph does not present an incomplete dependency graph.

app/lib/linear_cli/cli/commands/issues/read.ex:87

  • Graph mode first calls Linear.issues/1, whose Issue.full_fields/0 includes both root relation connections as well as comments, labels, and other full-view data, and then Graph.build/2 fetches Linear.issue_relations/1 for 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 omit state, 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.
@ruby-automation

Copy link
Copy Markdown
Contributor Author

Rework Summary (Run 4)

Addressed all three minor items flagged in the code review:

1. ASCII + connector alignment fixed (display.ex)

Three rendering paths had off-by-one or off-by-two misalignment in junction connectors:

  • chain_succ/3: pad was root+succ+9, corrected to root+succ+8 so continuation + aligns with the + in the first line
  • render_columns/5 no-pred multi-succ: pad was root_len+1, corrected to root_len-1 (so len(pad)+4 = root_len+3, matching first-line + position)
  • render_columns/5 preds+multi-succ: succ_arrow returned a multi-line string that, when embedded in "#{padded} --+--> #{arrow}", lost the predecessor column offset entirely. Replaced succ_arrow call with mid_pred_line/4 dispatch that computes cont_pad = pred_max + root_len + 11 (derived from the actual position of the second + in the first line)

All formulas verified arithmetically and by rendering concrete examples.

2. AGENTS.md module structure updated

Added CLI.Commands.Issues.Graph — --graph transitive dependency graph builder after the Relations entry, per the maintenance convention.

3. Discovered node status assertion added to graph tests

Added assert Enum.find(graph.nodes, &(&1.identifier == "EXT-57")).status == "Todo" to the "single outbound blocks edge" test, closing the coverage gap for the get_in(endpoint, [:state, :name]) path in add_endpoint/5.

Quality gates: 518 tests pass, credo clean, pre-push hooks pass.

@ruby-automation
ruby-automation merged commit 95035b1 into main Sep 13, 2026
3 checks passed
@ruby-automation
ruby-automation deleted the EXT-56-graph-dependency branch September 13, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants