From bcbaa7fcb5b96b9034c5a5f97a0be99719905396 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Wed, 23 Sep 2026 10:50:17 +0000 Subject: [PATCH 1/3] fix: return each analyzed error once in analyze_error analyze_error appended the raw error string for every graph error node whose content did not match, so an error that matched an existing node was returned both as that node and as a string. Resolve each parsed error to its first matching node, or keep the string when none matches. Fixes #1475 --- .../coder/CoSTEER/knowledge_management.py | 13 ++- .../utils/coder/test_costeer_analyze_error.py | 88 +++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 test/utils/coder/test_costeer_analyze_error.py diff --git a/rdagent/components/coder/CoSTEER/knowledge_management.py b/rdagent/components/coder/CoSTEER/knowledge_management.py index 820c21381f..72f57ca4d8 100644 --- a/rdagent/components/coder/CoSTEER/knowledge_management.py +++ b/rdagent/components/coder/CoSTEER/knowledge_management.py @@ -519,13 +519,12 @@ def analyze_error( else: error_list = [] for error_content in error_contents: - for error_node in all_error_nodes: - if error_content == error_node.content: - error_list.append(error_node) - else: - error_list.append(error_content) - if error_list[-1] in error_list[:-1]: - error_list.pop() + matched_node = next( + (error_node for error_node in all_error_nodes if error_node.content == error_content), None + ) + error_item = error_content if matched_node is None else matched_node + if error_item not in error_list: + error_list.append(error_item) return error_list diff --git a/test/utils/coder/test_costeer_analyze_error.py b/test/utils/coder/test_costeer_analyze_error.py new file mode 100644 index 0000000000..aa1b73956b --- /dev/null +++ b/test/utils/coder/test_costeer_analyze_error.py @@ -0,0 +1,88 @@ +from types import SimpleNamespace + +import pytest + +from rdagent.components.coder.CoSTEER.knowledge_management import ( + CoSTEERRAGStrategyV2, +) +from rdagent.components.knowledge_management.graph import ( + UndirectedGraph, + UndirectedNode, +) + +ROWS_ERROR = "The source dataframe and the ground truth dataframe have different rows count." +TOLERANCE_ERROR = "Some values differ by more than the tolerance of 1e-6." +EXECUTION_FEEDBACK = ( + 'Traceback (most recent call last):\n File "factor.py", line 3, in \n' + " x = 1 / 0\nZeroDivisionError: division by zero" +) +EXECUTION_ERROR = "ErrorType: ZeroDivisionError\nError line: x = 1 / 0" + + +def _strategy(error_nodes: list[UndirectedNode]) -> CoSTEERRAGStrategyV2: + graph = UndirectedGraph() + graph.nodes = {node.id: node for node in error_nodes} + strategy = CoSTEERRAGStrategyV2.__new__(CoSTEERRAGStrategyV2) + strategy.knowledgebase = SimpleNamespace(graph=graph) + return strategy + + +@pytest.mark.offline +@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) +@pytest.mark.parametrize( + ("feedback", "feedback_type", "error_content"), + [ + pytest.param(ROWS_ERROR, "value", ROWS_ERROR, id="value"), + pytest.param(EXECUTION_FEEDBACK, "execution", EXECUTION_ERROR, id="execution"), + ], +) +def test_analyze_error_returns_matched_node_once( + feedback: str, feedback_type: str, error_content: str, matched_first: bool +) -> None: + matched = UndirectedNode(content=error_content, label="error") + unrelated = UndirectedNode(content="A different previous error.", label="error") + nodes = [matched, unrelated] if matched_first else [unrelated, matched] + + result = _strategy(nodes).analyze_error(feedback, feedback_type=feedback_type) + + assert len(result) == 1 + assert result[0] is matched + + +@pytest.mark.offline +@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) +def test_analyze_error_keeps_parsed_order(matched_first: bool) -> None: + matched = UndirectedNode(content=ROWS_ERROR, label="error") + unrelated = UndirectedNode(content="A different previous error.", label="error") + nodes = [matched, unrelated] if matched_first else [unrelated, matched] + strategy = _strategy(nodes) + + assert strategy.analyze_error(f"{ROWS_ERROR}\n{TOLERANCE_ERROR}", feedback_type="value") == [ + matched, + TOLERANCE_ERROR, + ] + assert strategy.analyze_error(f"{TOLERANCE_ERROR}\n{ROWS_ERROR}", feedback_type="value") == [ + TOLERANCE_ERROR, + matched, + ] + + +@pytest.mark.offline +def test_analyze_error_reports_repeated_error_once() -> None: + matched = UndirectedNode(content=ROWS_ERROR, label="error") + unrelated = UndirectedNode(content="A different previous error.", label="error") + + result = _strategy([unrelated, matched]).analyze_error(f"{ROWS_ERROR}\n{ROWS_ERROR}", feedback_type="value") + + assert len(result) == 1 + assert result[0] is matched + + +@pytest.mark.offline +def test_analyze_error_keeps_unmatched_error_as_string() -> None: + nodes = [ + UndirectedNode(content="A different previous error.", label="error"), + UndirectedNode(content="Yet another previous error.", label="error"), + ] + + assert _strategy(nodes).analyze_error(TOLERANCE_ERROR, feedback_type="value") == [TOLERANCE_ERROR] From ebf885455a9668714eed8cd398fc33b2f79d5dce Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:41:38 +0000 Subject: [PATCH 2/3] test: pin Undefined Error node and multi-node order in analyze_error --- .../utils/coder/test_costeer_analyze_error.py | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/test/utils/coder/test_costeer_analyze_error.py b/test/utils/coder/test_costeer_analyze_error.py index aa1b73956b..2769c68ceb 100644 --- a/test/utils/coder/test_costeer_analyze_error.py +++ b/test/utils/coder/test_costeer_analyze_error.py @@ -17,6 +17,7 @@ " x = 1 / 0\nZeroDivisionError: division by zero" ) EXECUTION_ERROR = "ErrorType: ZeroDivisionError\nError line: x = 1 / 0" +UNPARSED_FEEDBACK = "Execution timed out after 600 seconds." def _strategy(error_nodes: list[UndirectedNode]) -> CoSTEERRAGStrategyV2: @@ -68,21 +69,36 @@ def test_analyze_error_keeps_parsed_order(matched_first: bool) -> None: @pytest.mark.offline -def test_analyze_error_reports_repeated_error_once() -> None: - matched = UndirectedNode(content=ROWS_ERROR, label="error") +@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) +def test_analyze_error_returns_undefined_error_node_once(matched_first: bool) -> None: + matched = UndirectedNode(content="Undefined Error", label="error") unrelated = UndirectedNode(content="A different previous error.", label="error") + nodes = [matched, unrelated] if matched_first else [unrelated, matched] - result = _strategy([unrelated, matched]).analyze_error(f"{ROWS_ERROR}\n{ROWS_ERROR}", feedback_type="value") + result = _strategy(nodes).analyze_error(UNPARSED_FEEDBACK, feedback_type="execution") assert len(result) == 1 assert result[0] is matched @pytest.mark.offline -def test_analyze_error_keeps_unmatched_error_as_string() -> None: - nodes = [ - UndirectedNode(content="A different previous error.", label="error"), - UndirectedNode(content="Yet another previous error.", label="error"), - ] +@pytest.mark.parametrize("graph_order", ["parsed", "reversed"]) +def test_analyze_error_orders_matched_nodes_by_feedback(graph_order: str) -> None: + rows = UndirectedNode(content=ROWS_ERROR, label="error") + tolerance = UndirectedNode(content=TOLERANCE_ERROR, label="error") + nodes = [rows, tolerance] if graph_order == "parsed" else [tolerance, rows] + + result = _strategy(nodes).analyze_error(f"{ROWS_ERROR}\n{TOLERANCE_ERROR}", feedback_type="value") + + assert result == [rows, tolerance] + - assert _strategy(nodes).analyze_error(TOLERANCE_ERROR, feedback_type="value") == [TOLERANCE_ERROR] +@pytest.mark.offline +def test_analyze_error_reports_repeated_error_once() -> None: + matched = UndirectedNode(content=ROWS_ERROR, label="error") + unrelated = UndirectedNode(content="A different previous error.", label="error") + + result = _strategy([unrelated, matched]).analyze_error(f"{ROWS_ERROR}\n{ROWS_ERROR}", feedback_type="value") + + assert len(result) == 1 + assert result[0] is matched From b02e3891658ee6f467804c8da726bfe98a633f9d Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Fri, 25 Sep 2026 03:02:14 +0000 Subject: [PATCH 3/3] test: consolidate analyze_error regression cases Look the parsed error up with graph.find_node instead of a hand-rolled scan, and cover the three parse paths, matched-node order and the unmatched error position with one parametrized test plus two cases. --- .../coder/CoSTEER/knowledge_management.py | 4 +- .../utils/coder/test_costeer_analyze_error.py | 92 +++++-------------- 2 files changed, 25 insertions(+), 71 deletions(-) diff --git a/rdagent/components/coder/CoSTEER/knowledge_management.py b/rdagent/components/coder/CoSTEER/knowledge_management.py index 72f57ca4d8..c486e5aa69 100644 --- a/rdagent/components/coder/CoSTEER/knowledge_management.py +++ b/rdagent/components/coder/CoSTEER/knowledge_management.py @@ -519,9 +519,7 @@ def analyze_error( else: error_list = [] for error_content in error_contents: - matched_node = next( - (error_node for error_node in all_error_nodes if error_node.content == error_content), None - ) + matched_node = self.knowledgebase.graph.find_node(content=error_content, label="error") error_item = error_content if matched_node is None else matched_node if error_item not in error_list: error_list.append(error_item) diff --git a/test/utils/coder/test_costeer_analyze_error.py b/test/utils/coder/test_costeer_analyze_error.py index 2769c68ceb..c50a096ed3 100644 --- a/test/utils/coder/test_costeer_analyze_error.py +++ b/test/utils/coder/test_costeer_analyze_error.py @@ -12,93 +12,49 @@ ROWS_ERROR = "The source dataframe and the ground truth dataframe have different rows count." TOLERANCE_ERROR = "Some values differ by more than the tolerance of 1e-6." -EXECUTION_FEEDBACK = ( - 'Traceback (most recent call last):\n File "factor.py", line 3, in \n' - " x = 1 / 0\nZeroDivisionError: division by zero" -) +EXECUTION_FEEDBACK = 'File "factor.py", line 3, in \n x = 1 / 0\nZeroDivisionError: division by zero' EXECUTION_ERROR = "ErrorType: ZeroDivisionError\nError line: x = 1 / 0" -UNPARSED_FEEDBACK = "Execution timed out after 600 seconds." -def _strategy(error_nodes: list[UndirectedNode]) -> CoSTEERRAGStrategyV2: +def _error(content: str) -> UndirectedNode: + return UndirectedNode(content=content, label="error") + + +def _strategy(*nodes: UndirectedNode) -> CoSTEERRAGStrategyV2: graph = UndirectedGraph() - graph.nodes = {node.id: node for node in error_nodes} + graph.nodes = {node.id: node for node in nodes} strategy = CoSTEERRAGStrategyV2.__new__(CoSTEERRAGStrategyV2) strategy.knowledgebase = SimpleNamespace(graph=graph) return strategy @pytest.mark.offline -@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) @pytest.mark.parametrize( - ("feedback", "feedback_type", "error_content"), + ("feedback", "feedback_type", "content"), [ - pytest.param(ROWS_ERROR, "value", ROWS_ERROR, id="value"), - pytest.param(EXECUTION_FEEDBACK, "execution", EXECUTION_ERROR, id="execution"), + (ROWS_ERROR, "value", ROWS_ERROR), + (EXECUTION_FEEDBACK, "execution", EXECUTION_ERROR), + ("Execution timed out after 600 seconds.", "execution", "Undefined Error"), ], + ids=["value", "execution", "undefined"], ) -def test_analyze_error_returns_matched_node_once( - feedback: str, feedback_type: str, error_content: str, matched_first: bool -) -> None: - matched = UndirectedNode(content=error_content, label="error") - unrelated = UndirectedNode(content="A different previous error.", label="error") - nodes = [matched, unrelated] if matched_first else [unrelated, matched] - - result = _strategy(nodes).analyze_error(feedback, feedback_type=feedback_type) - - assert len(result) == 1 - assert result[0] is matched - - -@pytest.mark.offline -@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) -def test_analyze_error_keeps_parsed_order(matched_first: bool) -> None: - matched = UndirectedNode(content=ROWS_ERROR, label="error") - unrelated = UndirectedNode(content="A different previous error.", label="error") - nodes = [matched, unrelated] if matched_first else [unrelated, matched] - strategy = _strategy(nodes) - - assert strategy.analyze_error(f"{ROWS_ERROR}\n{TOLERANCE_ERROR}", feedback_type="value") == [ - matched, - TOLERANCE_ERROR, - ] - assert strategy.analyze_error(f"{TOLERANCE_ERROR}\n{ROWS_ERROR}", feedback_type="value") == [ - TOLERANCE_ERROR, - matched, - ] - - -@pytest.mark.offline -@pytest.mark.parametrize("matched_first", [True, False], ids=["matched_first", "matched_last"]) -def test_analyze_error_returns_undefined_error_node_once(matched_first: bool) -> None: - matched = UndirectedNode(content="Undefined Error", label="error") - unrelated = UndirectedNode(content="A different previous error.", label="error") - nodes = [matched, unrelated] if matched_first else [unrelated, matched] - - result = _strategy(nodes).analyze_error(UNPARSED_FEEDBACK, feedback_type="execution") - - assert len(result) == 1 - assert result[0] is matched +def test_analyze_error_returns_matched_node_once(feedback: str, feedback_type: str, content: str) -> None: + matched = _error(content) + strategy = _strategy(_error("A different previous error."), matched) + assert strategy.analyze_error(feedback, feedback_type=feedback_type) == [matched] @pytest.mark.offline @pytest.mark.parametrize("graph_order", ["parsed", "reversed"]) def test_analyze_error_orders_matched_nodes_by_feedback(graph_order: str) -> None: - rows = UndirectedNode(content=ROWS_ERROR, label="error") - tolerance = UndirectedNode(content=TOLERANCE_ERROR, label="error") - nodes = [rows, tolerance] if graph_order == "parsed" else [tolerance, rows] - - result = _strategy(nodes).analyze_error(f"{ROWS_ERROR}\n{TOLERANCE_ERROR}", feedback_type="value") - - assert result == [rows, tolerance] + rows, tolerance = _error(ROWS_ERROR), _error(TOLERANCE_ERROR) + strategy = _strategy(rows, tolerance) if graph_order == "parsed" else _strategy(tolerance, rows) + feedback = f"{ROWS_ERROR}\n{TOLERANCE_ERROR}\n{ROWS_ERROR}" + assert strategy.analyze_error(feedback, feedback_type="value") == [rows, tolerance] @pytest.mark.offline -def test_analyze_error_reports_repeated_error_once() -> None: - matched = UndirectedNode(content=ROWS_ERROR, label="error") - unrelated = UndirectedNode(content="A different previous error.", label="error") - - result = _strategy([unrelated, matched]).analyze_error(f"{ROWS_ERROR}\n{ROWS_ERROR}", feedback_type="value") - - assert len(result) == 1 - assert result[0] is matched +def test_analyze_error_keeps_unmatched_error_in_order() -> None: + rows = _error(ROWS_ERROR) + strategy = _strategy(_error("A different previous error."), rows) + assert strategy.analyze_error(f"{TOLERANCE_ERROR}\n{ROWS_ERROR}", feedback_type="value") == [TOLERANCE_ERROR, rows]