-
Notifications
You must be signed in to change notification settings - Fork 15
Description
Part of duplicate code analysis: #2085
Summary
internal/server/difc_log.go (added in commit 9a493ea) defines a FilteredItemLogEntry struct whose fields are a strict subset of logger.JSONLFilteredItem. The only difference is that JSONLFilteredItem adds Timestamp and Type fields. The toJSONLFilteredItem() method is a verbatim field-by-field copy of all 11 shared fields.
Duplication Details
Pattern: Near-Identical Struct Definitions with Verbatim Field Copy
-
Severity: Medium
-
Occurrences: 2 struct definitions + 1 conversion method
-
Locations:
internal/server/difc_log.go(lines 15–28):FilteredItemLogEntryinternal/logger/jsonl_logger.go(lines 145–160):JSONLFilteredIteminternal/server/difc_log.go(lines 76–91):toJSONLFilteredItem()conversion
-
Duplicated Fields (11 fields identical in both structs):
ServerID string `json:"server_id"` ToolName string `json:"tool_name"` Description string `json:"description"` Reason string `json:"reason"` SecrecyTags []string `json:"secrecy_tags"` IntegrityTags []string `json:"integrity_tags"` AuthorAssociation string `json:"author_association,omitempty"` AuthorLogin string `json:"author_login,omitempty"` HTMLURL string `json:"html_url,omitempty"` Number string `json:"number,omitempty"` SHA string `json:"sha,omitempty"`
-
Verbatim Conversion (15 lines that copy every field one-by-one):
func (e FilteredItemLogEntry) toJSONLFilteredItem() *logger.JSONLFilteredItem { return &logger.JSONLFilteredItem{ ServerID: e.ServerID, ToolName: e.ToolName, Description: e.Description, Reason: e.Reason, SecrecyTags: e.SecrecyTags, IntegrityTags: e.IntegrityTags, AuthorAssociation: e.AuthorAssociation, AuthorLogin: e.AuthorLogin, HTMLURL: e.HTMLURL, Number: e.Number, SHA: e.SHA, } }
Impact Analysis
- Maintainability: Any new identifying field (e.g.,
RepoOwner,Branch) must be added to both structs and the conversion method — three edits instead of one. - Bug Risk: A field added to
JSONLFilteredItembut forgotten inFilteredItemLogEntry(or the conversion) will silently produce incomplete JSONL log entries. - Code Bloat: ~30 lines of duplicate field declarations + 15-line conversion method that adds no transformation logic.
Refactoring Recommendations
Option A — Embed the shared data in JSONLFilteredItem (preferred)
Move the 11 shared fields into a new FilteredItemData struct in the logger package, then embed it in JSONLFilteredItem and use it directly in the server package:
// logger/jsonl_logger.go
type FilteredItemData struct {
ServerID string `json:"server_id"`
ToolName string `json:"tool_name"`
// ... remaining 9 fields ...
}
type JSONLFilteredItem struct {
Timestamp string `json:"timestamp"`
Type string `json:"type"`
FilteredItemData
}
// server/difc_log.go — no separate struct needed
func logFilteredItems(serverID, toolName string, filtered *difc.FilteredCollectionLabeledData) {
for _, detail := range filtered.Filtered {
data := buildFilteredItemData(serverID, toolName, detail)
// write text log from data ...
logger.LogDifcFilteredItem(&logger.JSONLFilteredItem{FilteredItemData: data})
}
}Option B — Build JSONLFilteredItem directly
Populate *logger.JSONLFilteredItem directly in buildFilteredItemLogEntry (eliminating FilteredItemLogEntry entirely) and format the text log line from the JSONL struct before calling LogDifcFilteredItem.
- Estimated effort: ~1 hour
- Benefits: Single source of truth for field set; field additions require one edit only
Implementation Checklist
- Review duplication findings
- Choose refactoring approach (A or B above)
- Implement changes in
internal/server/difc_log.goandinternal/logger/jsonl_logger.go - Verify text log output is unchanged
- Verify JSONL output is unchanged
- Run
make agent-finishedto confirm all tests pass
Parent Issue
See parent analysis report: #2085
Related to #2085
Generated by Duplicate Code Detector · ◷
- expires on Mar 25, 2026, 3:05 AM UTC