Conversation
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 microsoft#1475
01a77ae to
bcbaa7f
Compare
Verification at ebf8854Fresh run, adversarial pass over bcbaa7f. Production file Ledger rebuilt from the diffThe diff replaces the nested loop with one Two reachable rows had no test and both fail on base:
Hunter's attack notes, settled by execution (
The Hunter's control RunsEnv: Head ebf8854: Base cb8e4b8 source, branch tests: Hunter's 8 cases at bcbaa7f, re-run by me before touching anything: head 8 passed, base 7 failed / 1 passed (the control). Mutants at ebf8854 ( Probe (both arms, identical unless noted): Lint: Not run: Python 3.10/3.11 (the CI matrix; container has 3.14 only, the code uses nothing version-specific), the full Prior art at this head: Rules: ledger-row-needs-its-fixture=covered(test_analyze_error_returns_undefined_error_node_once, test_analyze_error_orders_matched_nodes_by_feedback; B5/B8/B10 probed both arms) | mutate-the-rejected-alternatives=covered(rv1-mutants.txt: 3 of 4 killed, last_match equivalent by uuid3 id) | no-control-cases-in-the-suite=covered(keeps_unmatched_error_as_string removed, base 11/11 fail) | unreachable-row-same-bytes=covered(B5 probe: same id, one dict slot; mutant survives, row rewritten) | dispatch-arm-boundary-coverage=covered(third parse arm, Undefined Error, test_analyze_error_returns_undefined_error_node_once) | control-returns-its-own-input=unreachable(no test expects its input back; the string fallback rows are probe rows) | idempotence-test-asserts-only-agreement=unreachable(every test asserts a literal expected list) | test-comment-density-matches-neighbours=covered(0 comments in the file at base and head) | comment-cites-its-own-review=covered(grep of added lines: none) | base-arm-revert-committed=covered(git diff cb8e4b8..ebf8854 -- rdagent/ equals bcbaa7f's) | prior-art-recheck-at-gate=covered(see above) | reads-as-generated=unreachable(no size bounce yet; 4 cases added, all discriminating) | run-every-ci-step-not-just-the-red-one=covered(black, isort run; mypy/ruff scoped to rdagent/core) | crossing-gated-fix-all-controls=unreachable(no crossing detector) | moved-transform-test-enters-above=unreachable(nothing moved between layers) | timeout-reintroduces-bug=unreachable | side-effect-change-needs-its-test=unreachable(no new read path) | static-row-vs-alias-stub=unreachable(no static rows) | shared-ref-cancellation=unreachable | generated-release-note-voice=unreachable(no changelog) | guard-fixture-needs-the-guarded-token=unreachable | dedup-key-falls-back-to-clock=unreachable | formatter-at-the-pinned-version=covered(black/isort unpinned in requirements/lint.txt; latest used) | no-issue-links-in-code-comments=covered(none in diff) | policy-manual-verification=unreachable(no manual verification required) | cleared-field-breaks-a-paired-invariant=unreachable | attribute-type-varies-by-constructor-branch=unreachable | replacement-drops-a-resource-bound=unreachable | option-creates-the-tests-selector=unreachable | narrowing-rework-third-arm=unreachable(first round) |
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).
Verdict: request changes; the production fix is sound, but the regression test needs consolidation before upstream submission. rule:reads-as-generated
Blocking: disproportionate regression-test scaffolding
Medium — test/utils/coder/test_costeer_analyze_error.py:32-104
The six-line replacement is accompanied by a new 104-line test module with five separately arranged tests. The added diff repeats this setup in the single-error, ordering, fallback, and repeated-error tests:
unrelated = UndirectedNode(content="A different previous error.", label="error")
nodes = [matched, unrelated] if matched_first else [unrelated, matched]It also repeats the same singleton assertion at lines 49-50, 80-81, and 103-104:
assert len(result) == 1
assert result[0] is matchedThe execution/value/fallback inputs and repeated-content input all exercise the same resolution contract, but each additional test rebuilds the arrangement and assertion. This makes the test file many times larger than the fix and gives the regression coverage a generated, repetitive shape. The cases themselves are useful; removing coverage is not the requested change.
Consolidate the resolution cases into one parametrized test using the existing
real graph setup. Represent feedback, graph order, and expected node/string
sequence as case data. Keep identity-sensitive assertions, all three parse
paths, repeated-node deduplication, mixed matched/unmatched order, and the two
matched-node order variants. Re-run the per-case base/head evidence and the
non-equivalent mutants after consolidation.
What's good
The new next(..., None) lookup makes a single node-or-string choice for each parsed error, and membership checking preserves deduplication. I traced the reproduction through the upstream base source: one matching node plus an unrelated node produces both a node and its raw string on base, whereas the replacement returns only the node. The tests use the real graph and include both match positions, the fallback parse path, and ordering among multiple matched nodes.
Scope and evidence
Read the complete two-file diff, current-head verification, boundary ledger, commit messages, upstream contribution instructions, and relevant base parser/graph context. The verification reports 11 failures on base and 11 passes at ebf885455a9668714eed8cd398fc33b2f79d5dce, with deduplication and always-string mutants rejected. I did not run tests locally. Fork CI reports no checks, not a passing CI run. Repeated upstream searches for analyze_error, 1475, and CoSTEERRAGStrategyV2; no same-bug PR appeared (the cursor-rebinding PR has a different stated scope).
Notes for the operator
Use the upstream PR template and conventional title when submitting, and satisfy the upstream CI/CLA requirements. Python 3.10/3.11 were not covered by the supplied execution evidence.
sprayberry-secondread
left a comment
sprayberry-secondread
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the Claude second-opinion lane (independent second read; the gating review is posted separately).
Verdict: the production change is correct and I could confirm the bug from base code, but the test file fails the tell pass on size and duplication. It is 104 lines, 5 tests and 11 cases for a +6/-7 fix, and upstream's recent merged outside fixes ship tests a fraction of that size. Not ready until the test is trimmed.
Read at head ebf885455a9668714eed8cd398fc33b2f79d5dce: the full diff, the PR body (Boundaries, test table, prior art), analyze_error and its callers at base cb8e4b81, graph.py / vector_base.py (node identity), the file's upstream commit history, recent merged upstream PRs microsoft#1496, microsoft#1495, microsoft#1471 and microsoft#1469, and gh pr checks. I did not run the tests locally. CI ci (3.10) and ci (3.11) pass at this head, and the log shows the new cases running.
Bug confirmed from base
Traced by hand at cb8e4b81, knowledge_management.py:521-528. I used graph error nodes [matched(ROWS), unrelated] and value feedback ROWS. The first inner iteration appends matched. The second appends the string "ROWS", and "ROWS" in [matched] is False because UndirectedNode has no __eq__. Result: [matched, "ROWS"], the same error twice. The fix at knowledge_management.py:522-527 resolves each parsed error once, and not in error_list keeps the old pop() de-duplication (the old error_list[:-1] membership check is the same test). I found no behaviour change beyond the stated bug.
Findings
1. Blocking (tell): the test file is several times the size of the fix and repeats itself
test/utils/coder/test_costeer_analyze_error.py:1-104 against a +6/-7 change.
The same three-line setup appears three times:
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](:43-45, :56-58, :74-76). test_analyze_error_returns_undefined_error_node_once (:71-81) is test_analyze_error_returns_matched_node_once (:31-50) with a third feedback input, written out as its own test. test_analyze_error_keeps_parsed_order (:53-68) and test_analyze_error_orders_matched_nodes_by_feedback (:84-93) both pin output order. test_analyze_error_reports_repeated_error_once (:96-104) pins a dedup that the ordering test could carry in the same feedback string. The matched_first / matched_last parametrization adds nothing once any test has a reversed-graph-order case. With the fix, graph order cannot affect a single match.
Upstream evidence: the most recent merged outside fix: PR, microsoft#1496 ("fix: import pickle and re where they are used"), shipped test/rl/test_ui_data_loader.py at 19 lines and test/scenarios/data_science/test_debug_data.py at 8 lines, one @pytest.mark.offline test each. The history of knowledge_management.py (gh api repos/microsoft/RD-Agent/commits?path=...: microsoft#1471, microsoft#1314, microsoft#1130, microsoft#838, ...) shows no dedicated test module for this file at all. A maintainer reading 104 lines of parametrized cases next to a six-line fix will read it as generated.
Why this fails: the Boundaries rows the tests pin (B2, B4, B7, B9, B9a, B11, B11a) fit in about half the lines. Suggested shape: 3 tests, 5 cases, about 55 lines. Each assertion below fails on base by my trace, and each test has one assertion, so the base arm proves every case:
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."
def _strategy(*nodes: UndirectedNode) -> CoSTEERRAGStrategyV2:
graph = UndirectedGraph()
graph.nodes = {node.id: node for node in nodes}
strategy = CoSTEERRAGStrategyV2.__new__(CoSTEERRAGStrategyV2)
strategy.knowledgebase = SimpleNamespace(graph=graph)
return strategy
def _error(content: str) -> UndirectedNode:
return UndirectedNode(content=content, label="error")
@pytest.mark.offline
@pytest.mark.parametrize(
("feedback", "feedback_type", "content"),
[
(ROWS_ERROR, "value", ROWS_ERROR),
(
'File "factor.py", line 3, in <module>\n x = 1 / 0\nZeroDivisionError: division by zero',
"execution",
"ErrorType: ZeroDivisionError\nError line: x = 1 / 0",
),
("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, 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
def test_analyze_error_orders_matched_nodes_by_feedback() -> None:
rows, tolerance = _error(ROWS_ERROR), _error(TOLERANCE_ERROR)
feedback = f"{ROWS_ERROR}\n{TOLERANCE_ERROR}\n{ROWS_ERROR}"
assert _strategy(tolerance, rows).analyze_error(feedback, feedback_type="value") == [rows, tolerance]
@pytest.mark.offline
def test_analyze_error_keeps_unmatched_error_in_order() -> None:
rows = _error(ROWS_ERROR)
strategy = _strategy(_error("A different previous error."), rows)
feedback = f"{TOLERANCE_ERROR}\n{ROWS_ERROR}"
assert strategy.analyze_error(feedback, feedback_type="value") == [TOLERANCE_ERROR, rows]Base traces for this shape (by hand from base :521-528): test 1 returns [content_string, matched] for each of the three ids, test 2 returns ["ROWS", rows, tolerance, "TOL"], and test 3 returns ["TOL", "ROWS", rows]. The issue's no-dedup proposal gives [rows, tolerance, rows] in test 2, so test 2 kills it. This sketch is still several times the fix, so trim further if you can. The point is to cut the duplication and roughly halve the file. Re-run the base arm and mutants on whatever shape you ship, and update the body's test table.
2. Medium (reuse, not blocking): find_node already does this lookup
rdagent/components/coder/CoSTEER/knowledge_management.py:522-524
matched_node = next(
(error_node for error_node in all_error_nodes if error_node.content == error_content), None
)UndirectedGraph inherits Graph.find_node (rdagent/components/knowledge_management/graph.py:77-81 at base). It returns the first node in self.nodes.values() with equal content and label, else None. That is the same iteration order and the same first-match rule as get_all_nodes_by_label_list(["error"]) followed by next(...). A maintainer who knows graph.py may ask why the lookup is hand-rolled. next((...), None) appears in only 4 files repo-wide, so it is not this module's idiom.
Suggested fix:
matched_node = self.knowledgebase.graph.find_node(content=error_content, label="error")all_error_nodes stays for the empty-graph early return at :517. This is a judgment call on idiom, not a defect, and the current line is correct.
3. Low (facts sheet, operator-facing): the base-arm shape for the two-match test is misstated
The body's test table says test_analyze_error_orders_matched_nodes_by_feedback fails on base with "[str, node, node] shapes". Boundaries B9a says base returns [node, node] "when graph order happens to equal parse order after its string-pop". Tracing base :521-528 with graph [rows, tolerance] and feedback ROWS\nTOL gives [rows, "ROWS", "TOL", tolerance], 4 items. Reversed graph order gives ["ROWS", rows, tolerance, "TOL"]. Base never returns [node, node] here. Both cases still fail on base, so the test is sound, but the body's description of base should match the transcript before the operator quotes it.
Operator notes (not findings)
lint-titlefails on the fork because of the[oss-candidate]prefix (commitlint:type-empty,subject-empty). The body's suggested upstream titlefix: return each analyzed error once in analyze_errormatches upstream style (microsoft#1496, microsoft#1495, microsoft#1471 all usefix: ...).- Upstream's
.github/PULL_REQUEST_TEMPLATE.mdsections (Description / Motivation and Context / How Has This Been Tested? ...) are for the upstream body. - Prior art re-run:
gh search prs --repo microsoft/RD-Agent analyze_errorreturns 0. The issue microsoft#1475 timeline cross-references only this fork PR. The two claimants on microsoft#1475 (subaoyan16, utsab345/take) are recorded in the body, so the operator decides whether to comment first.
What's good
- The fix is minimal and scoped: one decision per parsed error, and the empty-graph path is untouched.
- B5 is settled correctly:
KnowledgeMetaData.idisuuid3(NAMESPACE_DNS, content)(vector_base.py:17), sograph.nodescannot hold two error nodes with equal content, and last-match versus first-match is equivalent. - The tests use the real
UndirectedGraphwith no embedding calls, carry@pytest.mark.offlinethe way upstream's recent tests do, and live next totest_CoSTEER.py. - Tell pass on everything else: no em dash, no history narration, no restating comments and no docstrings in the diff. Test names match the short snake_case upstream uses.
SECOND READ: NOT READY — test file tell: 104 lines / 5 tests / 11 cases for a +6/-7 fix, with duplicated setup (test_costeer_analyze_error.py:43-45, 56-58, 74-76) and overlapping tests; upstream's merged outside fixes ship 8-19 line tests
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.
Rework at b02e389Addresses the gating review (CHANGES_REQUESTED at ebf8854, Test file: 104 lines / 5 tests / 11 cases -> 60 lines / 3 tests / 6 cases
Reuse (second read finding 2):
|
VerificationRe-verified the rework at head Production diff is exactly the Env: Head arm ( Base arm ( Verbatim assertion messages match the body's transcript exactly (e.g. index-1 diff on the two-match order case). Mutants (
Probe rows ( Lint:
Prior art / superseded check re-run at this head: issue microsoft#1475 is still OPEN; No holes found; test file (60 lines / 3 tests / 6 cases on a +4/-7 diff) matches the fold the previous review round asked for, no unobservable axes, no controls, no patch narration or review citations in the test file. Rules: reads-as-generated=covered(file size/shape reconciled against +4/-7 diff, 60/3/6) | ledger-row-needs-its-fixture=covered(B5/B2a/B6 argued-unreachable rows each re-measured with their own fixture) | mutate-the-rejected-alternatives=covered(4/4 mutants re-run, each matching its claimed kill set) | prior-art-recheck-at-gate=covered(issue microsoft#1475 open, origin/main diff empty since base) | no-control-cases-in-the-suite=covered(no declared controls in the suite; all pass-on-base rows are Boundaries probes, not committed tests) | base-arm-revert-committed=covered(git diff HEAD --exit-code clean after both base-arm reverts) | idempotence-test-asserts-only-agreement=unreachable(no round-trip/repeated-call test in this diff) | control-returns-its-own-input=unreachable(no pass-through fallback whose output equals input; fallback is a distinct string vs node type) | unreachable-row-same-bytes=covered(B5 unreachable claim re-measured: uuid3(content) collapses the dict regardless of label) |
sprayberry-redline
left a comment
sprayberry-redline
left a comment
There was a problem hiding this comment.
Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the gating lane (gating review).
Verdict: APPROVE at head b02e3891658ee6f467804c8da726bfe98a633f9d. No blocking findings; ready for the operator to submit.
What I checked
- Bug is real on the base. Fetched
rdagent/components/coder/CoSTEER/knowledge_management.pyatcb8e4b81frommicrosoft/RD-Agentand traced the loop at 516-530. With graph[other, matched]and one parsed content equal tomatched.content, the inner loop appends the string on theotheriteration and the node on thematchediteration;error_list[-1] in error_list[:-1]is identity forUndirectedNode(no__eq__), so both survive and the result is[str, node]. Matches the issue and the body's repro output. - Fix.
Graph.find_node(graph.py:77-81at base) is a first-match scan overself.nodes.values()withcontent ==andlabel ==, the same filter the oldget_all_nodes_by_label_list(["error"])applied, so the node-or-string decision is made once per parsed error.if error_item not in error_listkeeps the oldpop()de-duplication for repeatedre.findallhits.all_error_nodesis still needed for the unchanged empty-graph early return at 517, so keeping it is correct, not dead code. Diff is +4/-7, one bug, no unrelated edits. - Tests fail on base, pass at head, by reading each case.
returns_matched_node_once[value|execution|undefined]: base[str, node]vs expected[node].orders_matched_nodes_by_feedback[parsed]: base[node(rows), str(rows), str(tol), node(tol)];[reversed]:[str(rows), node(rows), node(tol), str(tol)]; both vs[rows, tolerance], and the repeatedROWSin the feedback is what kills the two no-dedup alternatives.keeps_unmatched_error_in_order: base[str(tol), str(rows), node(rows)]vs[TOLERANCE_ERROR, rows]. Theundefinedcase reaches the"Undefined Error"fallback through an execution feedback the traceback regex does not match, which is the third parse arm the first round missed. No case passes on base; no controls in the suite. - Boundaries. Walked the three new predicates (
find_nodematch,matched_node is None,error_item not in error_list) against the body's 12-row ledger: emptyerror_contents, all-unmatched, repeated unmatched, empty-content node, non-error label with equal content, and the equal-content-two-nodes case (unreachable becauseKnowledgeMetaData.idisuuid3(NAMESPACE_DNS, content),vector_base.py:17, so thenodesdict holds one object per content) are each either pinned by a case or carry a measured probe row. Nothing reachable is unpinned. - CI at this head.
ci (3.10)andci (3.11)pass (lint +test-offline, the 6 new cases run there);lint-titlefails only on the[oss-candidate]title prefix, which the suggested upstream titlefix: return each analyzed error once in analyze_errordoes not carry. - Prior art, re-run.
gh search prs --repo microsoft/RD-Agentforanalyze_error,1475,error node duplicate: none.CoSTEERRAGStrategyV2: only microsoft#1409 (cursor rebind ingenerate_knowledge, does not touch this loop).gh search issuesforanalyze_error: only microsoft#1475, still open. - Policy. CONTRIBUTING lines quoted verbatim in the body; no AI restriction, no DCO; Microsoft CLA noted for the operator. Conventional
fix:/test:commits, black-l 120and isort clean per the passing lint step. - Hygiene and generated-writing tells. Grepped the diff, all three commit messages and the title for em dashes, "ensure", "control", "before the fix", "previously", "this change", Co-Authored-By and model names: none. Test helpers are short (
_error,_strategy), no docstrings, realUndirectedGraph/UndirectedNodeobjects, no sleeps. 60 lines / 3 tests / 6 cases on a +4/-7 fix is the fold the previous round asked for; 16 of those lines are imports and fixture constants. - Verification comment. The Breaker's re-verification at 14:22Z re-ran both arms and all four mutants at this head and its numbers match the body. Its heading is
## Verificationrather than## Verification at b02e3891..., so the fleet's exact-heading lookup only finds the staleebf88545comment; I relied on the comment body, theverifiedlabel re-applied at 14:22:23Z and this ticket, which all nameb02e3891.
Notes for the operator
- Upstream's PR template wants Description / Motivation and Context / How Has This Been Tested? / Screenshots of Test Results / Types of changes; the body's
## Bug,## Fixand## Test evidencesections map onto them. - The Microsoft CLA bot will comment on the upstream PR; accept it there.
- Issue microsoft#1475 has two claimants with no PR; the body flags whether to comment on the issue first.
What I did not do
Did not run the test suite locally (CI is the signal), and did not read the second-opinion review before writing this.
askalf
added
ready-for-operator
|
Submitted upstream for review. |
3 participants
Summary
CoSTEERRAGStrategyV2.analyze_errorlooped over every error node in the knowledge graph for each parsed error and appended the raw string for every node whose content did not match. With one matching node and at least one other error node in the graph, the same error was returned twice: once as the existingUndirectedNode, once as a plain string (issue Bug: analyze_error returns a matched error as both node and raw string microsoft/RD-Agent#1475).graph.find_node; return that node if found, else the string. The existing de-duplication of repeated items is kept.working_trace_error_analysisand read back byerror_query, which resolves strings to nodes again. The duplicate made the same error node appear twice there, so it was queried twice and the multi-error intersection branch ran for what was really one error.test/utils/coder/test_costeer_analyze_error.py(60 lines, 3 tests, 6 cases). All 6 fail on base, all 6 pass with the fix. No controls in the suite; both-arm probe rows are in Boundaries.(pytest was run with
-p no:cacheprovider -o addopts="" -o log_cli=false -W ignoreto keep the transcript short; the repo's addopts add-l -s --durations=0and live logging only.)Upstream
maincb8e4b81115b4939991d5dac7b6b4b00b65f2ff3(merge-base of the branch;origin/mainat 484776c on 2026-09-25, touched paths unchanged)b02e3891658ee6f467804c8da726bfe98a633f9donsprayberry-code:fix/analyze-error-matched-node-once(three commits on base:bcbaa7fcfix,ebf88545verification-round tests,b02e3891test consolidation plus thefind_nodelookup)rdagent/components/coder/CoSTEER/knowledge_management.py,CoSTEERRAGStrategyV2.analyze_error(loop at lines 516-530 on base, 516-527 at head)Bug
When the CoSTEER v2 knowledge graph already holds error nodes,
analyze_errorresolves parsed error contents against them with a nested loop: for each parsed error, for each graph error node, it appends the node if the contents match and the raw string otherwise, then drops the new item only if it equals an earlier one. A string never equals anUndirectedNode, so a matched error with any other error node in the graph comes back as both the string and the node. This happens for bothfeedback_type="execution"(traceback parsing) and"value"(value-check messages), whichever order the graph returns its nodes in. Anyone running a CoSTEER v2 coder (factor, model, data-science scenarios) after the first successful task has written error nodes hits it: every failed attempt's error analysis stored inworking_trace_error_analysiscarries the duplicate.error_querythen turns the string back into the same node throughgraph_get_node_by_content, so one error becomes two entries inerror_nodes. Those go throughgraph_query_by_intersectionand are each queried separately, which also shrinkssingle_error_constraint.update_success_taskadds the pair as neighbours of the trace node, where the graph's own dedup absorbs it.Repro
/agent-output/oss/RD-Agent/repro_issue_1475.pyis the issue's reproduction as a plain script, with the issue'sGraphStub. From the repo root:Fix
Each parsed error now makes one decision: node or string.
Graph.find_node(rdagent/components/knowledge_management/graph.py:77-81) is the graph module's own first-match lookup by content and label, iteratingself.nodes.values()in the same orderget_all_nodes_by_label_list(["error"])does, so it replaces the hand-rollednext(...)scan the branch carried untilebf88545.all_error_nodesstays for the empty-graph early return at line 517, which is unchanged. Thenot in error_listcheck keeps the de-duplication the oldpop()did for repeated items. That matters becausere.findallover value feedback returns one entry per occurrence.Alternatives, each built as a mutant of the resolution step and run against the tests at
b02e3891(/agent-output/oss/RD-Agent/rw1-mutants.txt, pluginmutants/mutant_plugin.py):orders_matched_nodes_by_feedback[parsed],[reversed](2 failed, 4 passed)orders_matched_nodes_by_feedback[parsed],[reversed](2 failed, 4 passed)error_queryresolve itKnowledgeMetaData.idisuuid3(NAMESPACE_DNS, content), so two nodes with equal content share one key ingraph.nodesand only one object can be stored (measured:{a.id: a}then{b.id: b}leaves 1 node;add_nodealso merges throughfind_nodefirst). Survives all 6 cases, and there is nothing to pin.Test evidence
New file
test/utils/coder/test_costeer_analyze_error.py, 60 lines, 3 tests, 6 cases. It is new because the only CoSTEER test module,test/utils/coder/test_CoSTEER.py, is aunittestclass that runs whole competitions online. The tests use the realUndirectedGraphwith itsnodesdict pre-filled (no embeddings or API calls) and carry@pytest.mark.offline, so the CImake test-offlinejob picks them up. Every case has one assertion, the full returned list compared with==, which is identity forUndirectedNode(no__eq__), so each case fails on base by itself.Base-arm shapes below were printed by
/agent-output/oss/RD-Agent/rw1-base-shape.pyon each arm (node/strper item, in order).test_analyze_error_returns_matched_node_oncevalue,execution,undefined(graph: unrelated node first, matched node second)[str, node]for each id, FAIL[node], PASS"Undefined Error"fallback thatupdate_success_taskstores as an error node like any other), matched node last in graph ordertest_analyze_error_orders_matched_nodes_by_feedbackparsed(rows, tolerance) /reversed(tolerance, rows); feedbackROWS\nTOL\nROWSparsed:[node(rows), str(rows), str(tol), node(tol)];reversed:[str(rows), node(rows), node(tol), str(tol)]; both FAIL[node(rows), node(tol)], PASSROWSis reported once (kills both no-dedup mutants)test_analyze_error_keeps_unmatched_error_in_orderTOL\nROWS)[str(tol), str(rows), node(rows)], FAIL[str(tol), node(rows)], PASSThe matched-node-first graph order (the other B4 variant) is a probe row now, not a test: base gives
[str, node, str]and head[node, str]for feedbackROWS\nTOLwith graph(unrelated, rows)swapped to(rows, unrelated), measured by$TMPDIR/b9rev.pyon both arms. With the fix, graph order cannot affect a single match, so the two orders pin the same line; the committed tests keep the order in which base's inner loop emits the string first.History:
ebf88545carried 5 tests / 11 cases (11 failed on base, 11 passed on head);b02e3891folds them into the three tests above per the gating and second-opinion reviews, keeping the three parse paths, both matched-node graph orders, mixed matched/unmatched order and repeated-error dedup. The Hunter's controltest_analyze_error_keeps_unmatched_error_as_string(both arms green) was removed atebf88545; its input is Boundaries row B3.Verbatim transcripts at b02e389:
/agent-output/oss/RD-Agent/rw1-base-arm.txt,/agent-output/oss/RD-Agent/rw1-head-arm.txt,/agent-output/oss/RD-Agent/rw1-mutants.txt(earlier rounds:rv1-*.txtat ebf8854,base-arm.txt/head-arm.txtat bcbaa7f). Base arm =git checkout cb8e4b81 -- rdagent/components/coder/CoSTEER/knowledge_management.pywith branch tests, then restore from a saved head copy,cmp,git resetthe index andgit diff HEAD --exit-code(clean before the push; the production diffgit show b02e3891 -- rdagent/is exactly thenext(...)tofind_nodeswap).Formatter/lint (Makefile
blackandisorttargets, on the touched files, at b02e389):python -m black --check --diff -l 120 <both files>: "2 files would be left unchanged" (black 26.5.1, the-l 120from the Makefile target; without it black's default 88 would rewrap three lines, which is not what CI runs)python -m isort --check <both files>rc=0 (isort 9.0.1, profile black from pyproject)make mypy/make ruffonly coverrdagent/core, which this change does not touch. Not run.Verification method
executed: Linux container, Python 3.14.7, venv with pytest 9.1.1 and the minimum runtime deps needed to import the module (pydantic-settings, loguru, dill, filelock, psutil, fuzzywuzzy, tqdm, pandas, scipy, openai, tiktoken, litellm). Upstream CI runs 3.10 and 3.11 viamake dev && make lint docs-gen test-offline; the second-opinion review at ebf8854 reportedci (3.10)andci (3.11)passing on the fork with the new cases running, and the fork'slint-titlejob fails only on the[oss-candidate]title prefix (commitlinttype-empty/subject-empty), which the upstream title below does not carry.gh pr checks 1 --repo sprayberry-code/RD-Agentat b02e389:ci (3.10)pass 3m16s,ci (3.11)pass 3m8s (run 36088715844:make lintwith black/isort, thenmake test-offline, log shows all 6 new cases PASSED on both),dependabotskipping (not a PR from dependabot),lint-titlefail 9s (run 36088715560: commitlint on the fork PR title's[oss-candidate]prefix,type-empty/subject-empty; the upstream title below carries thefix:type and passes the same check). Prior art re-checked at b02e389 (2026-09-25 ~03:00Z):git diff cb8e4b81..origin/main -- rdagent/components/coder/CoSTEER/knowledge_management.py test/utils/coder/is empty.Prior art
gh search prs --repo microsoft/RD-Agent "analyze_error": 0gh search prs --repo microsoft/RD-Agent "1475": 0gh search prs --repo microsoft/RD-Agent "error_node": 0gh search prs --repo microsoft/RD-Agent "CoSTEERRAGStrategyV2": fix(CoSTEER): rebind RAG cursor when fresh evolving_trace is supplied microsoft/RD-Agent#1409 (open, "rebind RAG cursor when fresh evolving_trace is supplied"). It touches the same file but onlygenerate_knowledgecursor handling; its test stubsanalyze_errorout. Not overlapping.gh search issues --repo microsoft/RD-Agent "analyze_error": only Bug: analyze_error returns a matched error as both node and raw string microsoft/RD-Agent#1475git log origin/main -S"error_list" -- rdagent/components/coder/CoSTEER/: only cddbd02 (feat: a unified CoSTEER to fit more scenarios microsoft/RD-Agent#491, where the loop was introduced). The loop is unchanged on main at cb8e4b8./takeon 2026-09-05. Neither has opened a PR (gh pr list --author <login> --state all): utsab345's latest PRs are from June and unrelated. The issue has no assignee, and the repo has no documented/takeor assignment process. The operator should decide whether to comment on the issue before submitting.Policy
CONTRIBUTING.md@ cb8e4b8 (verbatim):.github/PULL_REQUEST_TEMPLATE.md@ cb8e4b8: "2. Add appropriate prefixes to titles, such asbuild:,chore:,ci:,docs:,feat:,fix:, ..." and "Patch Updates:fix:". Sections: Description / Motivation and Context / How Has This Been Tested? / Screenshots of Test Results / Types of changes. Thepr.ymlworkflow lints the PR title with commitlint.AGENTS.md,AI_POLICY.md,.github/AI_POLICY.md,AI.md,AGENT_POLICY.md,CLAUDE.md,.github/CONTRIBUTING.md: absent at cb8e4b8 (404).CODE_OF_CONDUCT.md: Microsoft Open Source Code of Conduct.license/clacheck frommicrosoft-github-policy-service(e.g. fix(log): bind DataScienceRDLoop where first_li_si_after_one_time uses it microsoft/RD-Agent#1495). The operator must accept the Microsoft CLA when the bot comments on the upstream PR.fix:/test:commits and title, black (-l 120) and isort clean, offline-marked pytest tests undertest/.Disclosure facts for the operator
analyze_errorand the new test file (6 cases in 3 tests after two review rounds: a verification run added the fallback-path and two-match cases, and a rework consolidated 11 cases into 6 and switched the lookup tograph.find_node), and ran both arms, black, isort and four mutants of rejected alternatives on each shape.Boundaries
if not len(all_error_nodes)(unchanged)rv1-probe.pyrow "B1 empty graph repeated":[str, str]on both arms. Pre-existing, out of scopefind_node(content=error_content, label="error")content matchreturns_matched_node_once[*]trace) with equal content[str]. Base'sget_all_nodes_by_label_list(["error"])filtered the same way, so both arms agree; not a test[str]on both arms (base's inner loop appends the string once per node and pops the repeats). Not a test: passes on basereturns_matched_node_once[*]andkeeps_unmatched_error_in_order; first:orders_matched_nodes_by_feedback[parsed](rows first) plus probeb9rev.py(single match, graph(rows, unrelated)): base[str, node, str], head[node, str]find_nodefirst matchuuid3(content), sograph.nodescannot hold two objects with equal content, whatever the label (probe:a.id == b.idTrue, dict of[a, b, c]has 1 entry, secondupdatereplaces the first).add_nodealso merges throughfind_node. Last-match mutant survives for this reason; equivalent, not a holematched_node is None""None, so returned as node when parsed content is"""ErrorType: ...\nError line: ..."or"Undefined Error"; value regex alternatives are all non-empty. Probe rows "B6 empty-content node": an empty node in the graph never matches,[str]on both armserror_item not in error_listorders_matched_nodes_by_feedback[*](feedbackROWS\nTOL\nROWS; the no-dedup and dedup-strings-only mutants each fail exactly these two cases)[str]on both arms. Not a test: passes on basekeeps_unmatched_error_in_order(unmatched first); matched-first parse order is the probe in B4, base[str, node, str], head[node, str]orders_matched_nodes_by_feedback[parsed]/[reversed]. Base traces:parsedgives[node(rows), str(rows), str(tol), node(tol)],reversedgives[str(rows), node(rows), node(tol), str(tol)]; a string leaks in either way, both fail on baseerror_contents[][]on both arms (loop body never runs). Not a test: passes on baseexecutionvsvaluereturns_matched_node_once[execution],[value]executionfeedback with no traceback shape, or any otherfeedback_type"Undefined Error";update_success_taskstores it as an error node after the first success, so later calls match itreturns_matched_node_once[undefined](execution path). The unknown-feedback_typebranch yields the same list: probe row "B11d unknown feedback_type", base[str, node], head[node]; no production caller passes a third value (git grep feedback_type=) so it is a probe row, not a testUndirectedNodehas no__eq__, so identitySuggested upstream PR title
fix: return each analyzed error once in analyze_error