Description
PR #474 removed the per-task timeout logic from executeReviewFilter and ReLocateComment, along with the "timeout" values in task_template.json. However, the Timeout struct fields and the copy logic were left behind as dead code.
Currently the code still:
- Declares
Timeout int on both manifestConversation and LlmConversation structs
- Copies the (always-zero) value in
resolveConversation: conv := LlmConversation{Timeout: m.Timeout}
- Asserts on the zero value in
template_test.go
Since there is no custom-template loading mechanism (templates are //go:embed-only), these fields serve no purpose.
Scope
- File:
internal/config/template/template.go
- Remove
Timeout int from manifestConversation (line 57)
- Remove
Timeout int from LlmConversation (line 217)
- Simplify
resolveConversation to not copy Timeout (line 73)
- File:
internal/config/template/template_test.go
- Remove the
Timeout == 0 assertion
Acceptance Criteria
Context
Discovered during review of #474. The PR intentionally left struct fields in place to keep scope minimal; this issue tracks the follow-up cleanup.
Description
PR #474 removed the per-task timeout logic from
executeReviewFilterandReLocateComment, along with the"timeout"values intask_template.json. However, theTimeoutstruct fields and the copy logic were left behind as dead code.Currently the code still:
Timeout inton bothmanifestConversationandLlmConversationstructsresolveConversation:conv := LlmConversation{Timeout: m.Timeout}template_test.goSince there is no custom-template loading mechanism (templates are
//go:embed-only), these fields serve no purpose.Scope
internal/config/template/template.goTimeout intfrommanifestConversation(line 57)Timeout intfromLlmConversation(line 217)resolveConversationto not copyTimeout(line 73)internal/config/template/template_test.goTimeout == 0assertionAcceptance Criteria
Timeoutfield removed from bothmanifestConversationandLlmConversationresolveConversationno longer referencesTimeoutTimeoutremovedmake test)make check)Context
Discovered during review of #474. The PR intentionally left struct fields in place to keep scope minimal; this issue tracks the follow-up cleanup.