fix(copilot): address review comments on SDM validator and tests

- Filter out AgentInput/OutputBlocks from SDM tool link check (interface
  blocks aren't real tools)
- Add test for interface-block-only links failing validation
- Add test for explicit None values being treated as missing in fixer
- Assert full graph validity in e2e pipeline test
This commit is contained in:
Zamil Majdy
2026-03-16 22:19:45 +07:00
parent f0af149f16
commit 5d527eab85
2 changed files with 56 additions and 1 deletions

View File

@@ -822,6 +822,8 @@ class AgentValidator:
valid = True
nodes = agent.get("nodes", [])
links = agent.get("links", [])
node_lookup = {node.get("id", ""): node for node in nodes}
non_tool_block_ids = {AGENT_INPUT_BLOCK_ID, AGENT_OUTPUT_BLOCK_ID}
for node in nodes:
if node.get("block_id") != SMART_DECISION_MAKER_BLOCK_ID:
@@ -833,7 +835,10 @@ class AgentValidator:
)
has_tools = any(
link.get("source_id") == node_id and link.get("source_name") == "tools"
link.get("source_id") == node_id
and link.get("source_name") == "tools"
and node_lookup.get(link.get("sink_id", ""), {}).get("block_id")
not in non_tool_block_ids
for link in links
)

View File

@@ -222,6 +222,32 @@ class TestFixSmartDecisionMakerBlocks:
assert "input_default" in result["nodes"][0]
assert result["nodes"][0]["input_default"]["agent_mode_max_iterations"] == -1
def test_treats_none_values_as_missing(self):
"""Explicit None values are overwritten with defaults."""
fixer = AgentFixer()
agent = {
"nodes": [
_make_sdm_node(
input_default={
"agent_mode_max_iterations": None,
"conversation_compaction": None,
"retry": 3,
"multiple_tool_calls": False,
}
)
],
"links": [],
}
result = fixer.fix_smart_decision_maker_blocks(agent)
defaults = result["nodes"][0]["input_default"]
assert defaults["agent_mode_max_iterations"] == -1 # None → default
assert defaults["conversation_compaction"] is True # None → default
assert defaults["retry"] == 3 # kept
assert defaults["multiple_tool_calls"] is False # kept
assert len(fixer.fixes_applied) == 2
def test_multiple_sdm_nodes(self):
"""Multiple SDM nodes are all fixed independently."""
fixer = AgentFixer()
@@ -356,6 +382,27 @@ class TestValidateSmartDecisionMakerBlocks:
assert len(validator.errors) == 1
assert sdm_invalid["id"] in validator.errors[0]
def test_sdm_with_only_interface_block_links_fails(self):
"""Links to AgentInput/OutputBlocks don't count as tool connections."""
validator = AgentValidator()
sdm = _make_sdm_node()
input_node = _make_input_node()
output_node = _make_output_node()
agent = {
"nodes": [sdm, input_node, output_node],
"links": [
# These link to interface blocks, not real tools
_link(sdm["id"], "tools", input_node["id"], "name"),
_link(sdm["id"], "tools", output_node["id"], "value"),
],
}
result = validator.validate_smart_decision_maker_blocks(agent)
assert result is False
assert len(validator.errors) == 1
assert "no downstream tool blocks" in validator.errors[0]
def test_registered_in_validate(self):
"""validate_smart_decision_maker_blocks runs as part of validate()."""
validator = AgentValidator()
@@ -567,6 +614,9 @@ class TestSmartDecisionMakerE2EPipeline:
validator = AgentValidator()
is_valid, error_msg = validator.validate(fixed, blocks)
# Full graph validation should pass
assert is_valid, f"Validation failed: {error_msg}"
# SDM-specific validation should pass (has tool links)
sdm_errors = [e for e in validator.errors if "SmartDecisionMakerBlock" in e]
assert len(sdm_errors) == 0, f"Unexpected SDM errors: {sdm_errors}"