From 28fad41f36da63d07def285c6f2ae6f55f5778f0 Mon Sep 17 00:00:00 2001
From: GitHub Copilot <copilot@github.com>
Date: Thu, 19 Mar 2026 04:15:21 +0000
Subject: [PATCH] test: add coverage for parser errors, graph queries, and
 system roundtrip (8 new tests)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Parser error paths (integration_test.rs):
  - test_parse_error_numeric_literal_for_agent_name
  - test_parse_error_topic_missing_name
  - test_parse_with_errors_collects_errors_for_malformed_input

- Graph query functions (graph/queries.rs) — previously zero coverage:
  - test_find_usages_returns_referencing_nodes
  - test_find_action_invokers_returns_reasoning_action
  - test_get_topic_reasoning_actions_count
  - test_topic_execution_order_returns_none_for_cyclic_graph

- Serializer roundtrip (test_serializer_roundtrip.rs):
  - test_roundtrip_system_block

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/graph/queries.rs               | 125 +++++++++++++++++++++++++++++
 tests/integration_test.rs          |  43 +++++++++-
 tests/test_serializer_roundtrip.rs |  35 ++++++++
 3 files changed, 202 insertions(+), 1 deletion(-)

diff --git a/src/graph/queries.rs b/src/graph/queries.rs
index 7508e45..9c56d16 100644
--- a/src/graph/queries.rs
+++ b/src/graph/queries.rs
@@ -347,4 +347,129 @@ topic main:
         // At least one edge should exist (the Routes edge from start_agent → main)
         assert!(graph.edge_count() > 0, "Expected at least one edge in the graph");
     }
+
+    /// Source with a topic that defines an action def and a reasoning action that
+    /// invokes it via `@actions.<name>`.  Used to test Invokes-edge queries.
+    fn invocation_source() -> &'static str {
+        r#"config:
+   agent_name: "Test"
+
+start_agent selector:
+   description: "Route to main"
+   reasoning:
+      instructions: "Select"
+      actions:
+         go_main: @utils.transition to @topic.main
+            description: "Enter main"
+
+topic main:
+   description: "Main topic"
+
+   actions:
+      lookup:
+         description: "Look up a record"
+         inputs:
+            id: string
+               description: "Record ID"
+         outputs:
+            name: string
+               description: "Record name"
+         target: "flow://Lookup"
+
+   reasoning:
+      instructions: "Help"
+      actions:
+         do_lookup: @actions.lookup
+            description: "Perform the lookup"
+"#
+    }
+
+    /// Source with two topics that form a cycle: topic_a → topic_b → topic_a.
+    fn cyclic_two_topic_source() -> &'static str {
+        r#"config:
+   agent_name: "Test"
+
+start_agent selector:
+   description: "Route"
+   reasoning:
+      instructions: "Select"
+      actions:
+         go_a: @utils.transition to @topic.topic_a
+            description: "Go to A"
+
+topic topic_a:
+   description: "Topic A"
+   reasoning:
+      instructions: "In A"
+      actions:
+         go_b: @utils.transition to @topic.topic_b
+            description: "Go to B"
+
+topic topic_b:
+   description: "Topic B"
+   reasoning:
+      instructions: "In B"
+      actions:
+         go_a: @utils.transition to @topic.topic_a
+            description: "Back to A"
+"#
+    }
+
+    #[test]
+    fn test_find_usages_returns_referencing_nodes() {
+        // topic_b is referenced by topic_a's reasoning action via
+        // `@utils.transition to @topic.topic_b`.  find_usages(topic_b) should
+        // return at least that reasoning action node as an incoming edge source.
+        let graph = parse_and_build(two_topic_source());
+        let topic_b_idx = graph.get_topic("topic_b").expect("topic_b not found");
+        let usages = graph.find_usages(topic_b_idx);
+        assert!(
+            !usages.is_empty(),
+            "Expected at least one node referencing topic_b via an incoming edge"
+        );
+    }
+
+    #[test]
+    fn test_find_action_invokers_returns_reasoning_action() {
+        // The reasoning action `do_lookup` references `@actions.lookup`, creating
+        // a RefEdge::Invokes edge from the reasoning action node to the action-def
+        // node.  find_action_invokers(lookup_def) should return exactly that one
+        // reasoning action.
+        let graph = parse_and_build(invocation_source());
+        let lookup_idx = graph
+            .get_action_def("main", "lookup")
+            .expect("lookup action def not found");
+        let invokers = graph.find_action_invokers(lookup_idx);
+        assert_eq!(
+            invokers.len(),
+            1,
+            "Expected exactly 1 invoker for the lookup action def"
+        );
+    }
+
+    #[test]
+    fn test_get_topic_reasoning_actions_count() {
+        // topic 'main' defines exactly one reasoning action (`do_lookup`).
+        // get_topic_reasoning_actions("main") should return a vec of length 1.
+        let graph = parse_and_build(invocation_source());
+        let actions = graph.get_topic_reasoning_actions("main");
+        assert_eq!(
+            actions.len(),
+            1,
+            "Expected 1 reasoning action in topic 'main'"
+        );
+    }
+
+    #[test]
+    fn test_topic_execution_order_returns_none_for_cyclic_graph() {
+        // topic_a → topic_b → topic_a forms a cycle; topological sort is
+        // impossible for a directed cyclic graph.  topic_execution_order() must
+        // return None when the graph contains a cycle.
+        let graph = parse_and_build(cyclic_two_topic_source());
+        let order = graph.topic_execution_order();
+        assert!(
+            order.is_none(),
+            "Expected None for a cyclic graph, but topic_execution_order returned Some"
+        );
+    }
 }
diff --git a/tests/integration_test.rs b/tests/integration_test.rs
index f5db09b..fe29417 100644
--- a/tests/integration_test.rs
+++ b/tests/integration_test.rs
@@ -205,7 +205,7 @@ recipe_test!(
 
 #[cfg(test)]
 mod parser_unit_tests {
-    use busbar_sf_agentscript::{lexer, parse_source};
+    use busbar_sf_agentscript::{lexer, parse_source, parse_with_errors};
     use chumsky::prelude::*;
 
     #[test]
@@ -271,4 +271,45 @@ topic main:
             .collect();
         assert!(tokens.iter().any(|t| matches!(t, lexer::Token::Config)));
     }
+
+    #[test]
+    fn test_parse_error_numeric_literal_for_agent_name() {
+        // The `agent_name` field expects a string literal (`"..."`).  A numeric
+        // literal (123) is a different token type that `spanned_string()` rejects,
+        // so the parser should fail and return `Err`.
+        let source = "config:\n   agent_name: 123\n";
+        let result = parse_source(source);
+        assert!(
+            result.is_err(),
+            "Expected parse error for numeric agent_name, but parse succeeded"
+        );
+    }
+
+    #[test]
+    fn test_parse_error_topic_missing_name() {
+        // A `topic` keyword must be followed by an identifier (its name) and then
+        // a colon.  A bare `topic:` with the colon in the name position should
+        // trigger a parse error because `spanned_ident()` only matches `Ident`
+        // tokens, not `Colon`.
+        let source = "config:\n   agent_name: \"Test\"\n\ntopic:\n   description: \"No name\"\n";
+        let result = parse_source(source);
+        assert!(
+            result.is_err(),
+            "Expected parse error for topic without a name, but parse succeeded"
+        );
+    }
+
+    #[test]
+    fn test_parse_with_errors_collects_errors_for_malformed_input() {
+        // `parse_with_errors` returns both a partial AST and an error list.
+        // For clearly invalid input the error list must be non-empty, confirming
+        // that error collection works independently of whether recovery produces
+        // a partial tree.
+        let source = "config:\n   agent_name: 999\n";
+        let (_partial, errors) = parse_with_errors(source);
+        assert!(
+            !errors.is_empty(),
+            "Expected at least one error message for malformed config input"
+        );
+    }
 }
diff --git a/tests/test_serializer_roundtrip.rs b/tests/test_serializer_roundtrip.rs
index cc1c831..a550251 100644
--- a/tests/test_serializer_roundtrip.rs
+++ b/tests/test_serializer_roundtrip.rs
@@ -216,3 +216,38 @@ topic main:
     assert!(topic.before_reasoning.is_some(), "before_reasoning lost after roundtrip");
     assert!(topic.after_reasoning.is_some(), "after_reasoning lost after roundtrip");
 }
+
+#[test]
+fn test_roundtrip_system_block() {
+    // Covers the `system:` block — global instructions and welcome/error message
+    // templates.  The serializer writes this block but no roundtrip test existed.
+    // Ensures all system block content survives a parse → serialize → parse cycle.
+    let original = r#"config:
+   agent_name: "SupportAgent"
+
+system:
+   instructions: "You are a helpful support agent."
+   messages:
+      welcome: "Hello! How can I help you today?"
+      error: "I encountered an issue. Please try again."
+
+topic main:
+   description: "Main"
+"#;
+
+    let ast = parse(original).expect("Failed to parse original");
+    let serialized = serialize(&ast);
+
+    assert!(serialized.contains("system:"), "Missing system block in serialized output");
+    assert!(serialized.contains("instructions:"), "Missing instructions in serialized output");
+    assert!(serialized.contains("welcome:"), "Missing welcome message in serialized output");
+
+    let reparsed = parse(&serialized).expect("Failed to reparse serialized output");
+    assert!(reparsed.system.is_some(), "system block lost after roundtrip");
+    let sys = &reparsed.system.as_ref().unwrap().node;
+    assert!(sys.instructions.is_some(), "instructions lost after roundtrip");
+    assert!(sys.messages.is_some(), "messages block lost after roundtrip");
+    let messages = sys.messages.as_ref().unwrap();
+    assert!(messages.node.welcome.is_some(), "welcome message lost after roundtrip");
+    assert!(messages.node.error.is_some(), "error message lost after roundtrip");
+}