Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
halotukozak wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/alpaca/internal/parser/ConflictResolution.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,32 @@ private[parser] object ConflictResolutionTable:

for node <- table.keys do loop(Action.Enter(node) :: Nil)

def toMermaid(using Log): String =
val sb = new StringBuilder
sb.append("graph TD\n")
Comment on lines +107 to +109
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.

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")
Comment on lines +124 to +126
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.

for (from, toSet) <- table; to <- toSet do
Comment on lines +125 to +128
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.
sb.append(s" ${nodeName(from)} --> ${nodeName(to)}\n")
Comment on lines +111 to +129
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.

sb.toString

/**
* Showable instance for displaying conflict resolution tables.
*/
Expand Down
1 change: 1 addition & 0 deletions src/alpaca/internal/parser/createTables.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ private def createTablesImpl[Ctx <: ParserCtx: Type](
).tap: table =>
table.verifyNoConflicts()
logger.toFile(show"$parserName/conflictResolutions.dbg", true)(table)
logger.toFile(show"$parserName/conflictResolutions.mmd", true)(table.toMermaid)

logger.trace("Conflict resolution table built, identifying root production.")

Expand Down
Loading