What happened
In PR #2010, the review agent flagged data, _ := json.Marshal(payload) in internal/security/trace.go:55 as an error-handling gap (inline comment). The same finding appeared in the issue comment review as well. The author correctly pushed back: the payload struct contains only string fields, and json.Marshal cannot fail on a struct with no channels, functions, or unsupported types. The finding was surfaced twice across two review runs, creating redundant noise on a false positive.
What could go better
The review agent treats all ignored error returns as potential error-handling gaps regardless of whether the operation can actually fail. In Go, several stdlib functions are provably infallible for certain input types: json.Marshal on structs with only primitive/string fields, fmt.Sprintf (never errors), bytes.Buffer.Write (documented to always return nil error), hash.Write (documented to never return error). Flagging these creates false positives that erode trust in the review agent's error-handling findings. Confidence: high — this is a well-documented Go pattern, and the author's pushback was correct. The finding was low severity so it didn't block the PR, but it consumed author attention across two review rounds.
Proposed change
Update the review agent's code-quality or correctness sub-agent prompt (in the review agent definition or sub-agent definitions in the .fullsend dispatch repo) to include guidance recognizing common infallible Go stdlib operations. Specifically, add a note like: "When flagging ignored error returns in Go code, consider whether the operation is provably infallible for the given input types. Common examples: json.Marshal on structs with only primitive/string fields (no channels, functions, or interface types), bytes.Buffer.Write, hash.Write, fmt.Sprintf. Do not flag these as error-handling gaps unless the struct contains dynamic types that could cause marshaling failures." This belongs in the review agent's sub-agent prompt for correctness or code-quality analysis, not in repo-specific configuration, since the pattern is universal to Go codebases.
Validation criteria
On the next 5 Go PRs reviewed in fullsend-ai/fullsend that contain json.Marshal on simple structs with ignored errors, the review agent should not flag them as error-handling gaps. If the struct contains interface types, channels, or other types that can cause marshal failures, the agent should still flag the ignored error. Manual verification: search for review comments containing 'json.Marshal' and 'error' in the next 30 days of reviews.
Generated by retro agent from #2010
What happened
In PR #2010, the review agent flagged
data, _ := json.Marshal(payload)ininternal/security/trace.go:55as an error-handling gap (inline comment). The same finding appeared in the issue comment review as well. The author correctly pushed back: the payload struct contains only string fields, andjson.Marshalcannot fail on a struct with no channels, functions, or unsupported types. The finding was surfaced twice across two review runs, creating redundant noise on a false positive.What could go better
The review agent treats all ignored error returns as potential error-handling gaps regardless of whether the operation can actually fail. In Go, several stdlib functions are provably infallible for certain input types:
json.Marshalon structs with only primitive/string fields,fmt.Sprintf(never errors),bytes.Buffer.Write(documented to always return nil error),hash.Write(documented to never return error). Flagging these creates false positives that erode trust in the review agent's error-handling findings. Confidence: high — this is a well-documented Go pattern, and the author's pushback was correct. The finding was low severity so it didn't block the PR, but it consumed author attention across two review rounds.Proposed change
Update the review agent's code-quality or correctness sub-agent prompt (in the review agent definition or sub-agent definitions in the
.fullsenddispatch repo) to include guidance recognizing common infallible Go stdlib operations. Specifically, add a note like: "When flagging ignored error returns in Go code, consider whether the operation is provably infallible for the given input types. Common examples:json.Marshalon structs with only primitive/string fields (no channels, functions, or interface types),bytes.Buffer.Write,hash.Write,fmt.Sprintf. Do not flag these as error-handling gaps unless the struct contains dynamic types that could cause marshaling failures." This belongs in the review agent's sub-agent prompt for correctness or code-quality analysis, not in repo-specific configuration, since the pattern is universal to Go codebases.Validation criteria
On the next 5 Go PRs reviewed in fullsend-ai/fullsend that contain
json.Marshalon simple structs with ignored errors, the review agent should not flag them as error-handling gaps. If the struct contains interface types, channels, or other types that can cause marshal failures, the agent should still flag the ignored error. Manual verification: search for review comments containing 'json.Marshal' and 'error' in the next 30 days of reviews.Generated by retro agent from #2010