Skip to content

feat: debug conflict resolution graph printing (#150)#301

Open
halotukozak wants to merge 1 commit intomasterfrom
issue-150-mermaid-debug
Open

feat: debug conflict resolution graph printing (#150)#301
halotukozak wants to merge 1 commit intomasterfrom
issue-150-mermaid-debug

Conversation

@halotukozak
Copy link
Copy Markdown
Owner

Summary

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 4, 2026 14:49
@github-actions github-actions bot added enhancement New feature or request Parser labels Mar 4, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

🏃 Runtime Benchmark

Benchmark Base (master) Current (issue-150-mermaid-debug) Diff

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Mermaid graph output for the conflict resolution precedence graph to help debug/visualize conflict resolution rules (Issue #150).

Changes:

  • Emit a new conflictResolutions.mmd debug file during table creation.
  • Add ConflictResolutionTable.toMermaid to render the precedence graph in Mermaid format.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/alpaca/internal/parser/createTables.scala Writes Mermaid debug output alongside the existing conflict resolution debug table output.
src/alpaca/internal/parser/ConflictResolution.scala Implements Mermaid rendering for the conflict resolution graph.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +107 to +109
def toMermaid(using Log): String =
val sb = new StringBuilder
sb.append("graph TD\n")
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

toMermaid is new behavior and is now used to emit a debug artifact from createTablesImpl, but there’s no unit test coverage to ensure the output remains valid Mermaid (especially for token/production names with special characters) and stable over time. Adding a focused test in test/src/alpaca/internal/parser that builds a small ConflictResolutionTable and asserts key nodes/edges/escaping would help prevent regressions.

Copilot uses AI. Check for mistakes.
Comment on lines +111 to +129
def nodeName(key: ConflictKey): String =
val raw = key match
case p: Production => p.name match
case null => show"$p"
case name: String => name
case s: String => show"Token($s)"
// Escape special chars for mermaid
raw.replace(" ", "_").replace("(", "[").replace(")", "]").replace("->", "_to_").replace("ε", "epsilon")

def nodeLabel(key: ConflictKey): String = key match
case p: Production => show"$p"
case s: String => show"Token($s)"

val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")

for (from, toSet) <- table; to <- toSet do
sb.append(s" ${nodeName(from)} --> ${nodeName(to)}\n")
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

nodeName is used as the Mermaid node identifier, but the current “escaping” can still produce invalid IDs and collisions (e.g., replacing ( with [/] introduces Mermaid syntax chars; token/production names can contain many other non-identifier characters; production names can collide with token-derived names). Consider generating a safe, unique ID per ConflictKey (e.g., P_/T_ prefix + stable hash or an indexed map) and keep the human-readable text only in the label.

Suggested change
def nodeName(key: ConflictKey): String =
val raw = key match
case p: Production => p.name match
case null => show"$p"
case name: String => name
case s: String => show"Token($s)"
// Escape special chars for mermaid
raw.replace(" ", "_").replace("(", "[").replace(")", "]").replace("->", "_to_").replace("ε", "epsilon")
def nodeLabel(key: ConflictKey): String = key match
case p: Production => show"$p"
case s: String => show"Token($s)"
val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")
for (from, toSet) <- table; to <- toSet do
sb.append(s" ${nodeName(from)} --> ${nodeName(to)}\n")
// Generate safe, unique Mermaid node identifiers per ConflictKey.
val idMap = mutable.LinkedHashMap.empty[ConflictKey, String]
var prodIdx = 0
var tokIdx = 0
var otherIdx = 0
def nodeId(key: ConflictKey): String =
idMap.getOrElseUpdate(
key,
key match
case _: Production =>
prodIdx += 1
s"P_$prodIdx"
case _: String =>
tokIdx += 1
s"T_$tokIdx"
case _ =>
otherIdx += 1
s"N_$otherIdx"
)
def nodeLabel(key: ConflictKey): String = key match
case p: Production => show"$p"
case s: String => show"Token($s)"
val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeId(node)}[\"${nodeLabel(node)}\"]\n")
for (from, toSet) <- table; to <- toSet do
sb.append(s" ${nodeId(from)} --> ${nodeId(to)}\n")

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +126
val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

nodeLabel is injected into [...] with double quotes, but the label text isn’t escaped. Since token names (and production names) can include characters like ", ], or newlines, this can generate invalid Mermaid. Escape label content (at least \, ", and newlines) before writing it, or use a safer encoding for labels.

Suggested change
val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")
def escapeLabel(label: String): String =
label
.replace("\\", "\\\\")
.replace("\"", "\\\"")
.replace("\n", "\\n")
.replace("\r", "\\r")
val nodes = (table.keySet ++ table.values.flatten).toSet
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${escapeLabel(nodeLabel(node))}\"]\n")

Copilot uses AI. Check for mistakes.
Comment on lines +125 to +128
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")

for (from, toSet) <- table; to <- toSet do
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The Mermaid output iteration order is not deterministic (nodes is a Set and table is a Map), so the .mmd file may change between runs even when the underlying graph is the same. For debug artifacts that are intended to be diffed, consider sorting nodes/edges (e.g., by stable ID or label) before appending to the StringBuilder.

Suggested change
for node <- nodes do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")
for (from, toSet) <- table; to <- toSet do
// Sort nodes to ensure deterministic Mermaid output
for node <- nodes.toList.sortBy(nodeName) do
sb.append(s" ${nodeName(node)}[\"${nodeLabel(node)}\"]\n")
// Sort edges (both sources and targets) for deterministic ordering
for
(from, toSet) <- table.toList.sortBy { case (fromKey, _) => nodeName(fromKey) }
to <- toSet.toList.sortBy(nodeName)
do

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...rc/alpaca/internal/parser/ConflictResolution.scala 0.00% 18 Missing ⚠️
@@            Coverage Diff            @@
##             master     #301   +/-   ##
=========================================
  Coverage          ?   40.35%           
=========================================
  Files             ?       35           
  Lines             ?      451           
  Branches          ?        0           
=========================================
  Hits              ?      182           
  Misses            ?      269           
  Partials          ?        0           
Files with missing lines Coverage Δ
...rc/alpaca/internal/parser/ConflictResolution.scala 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2026

📊 Test Compilation Benchmark

Branch Average Time
Base (master) 52.061s
Current (issue-150-mermaid-debug) 51.158s

Result: Current branch is 0.903s faster (1.73%) ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants