steve/pansy
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

Agent: add the corrective tools the toolbox was missing (#85 item 3) #122

Merged
steve merged 2 commits from feat/agent-corrective-tools into main 2026-07-23 01:41:38 +00:00
Conversation 3 Commits 2 Files Changed 4
+17 -7
2 changed files with 17 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7b150275ae - Show all commits
internal/agent/tools.go
+6 -7
View File
@@ -2,7 +2,6 @@ package agent
import ( import (
"context" "context"
"time"
"gitea.stevedudenhoeffer.com/steve/majordomo/llm" "gitea.stevedudenhoeffer.com/steve/majordomo/llm"
1
 @@ -218,8 +217,9 @@ func (a *adapter) readJournal(ctx context.Context, args struct {
ObjectID *int64 `json:"objectId" description:"optional bed to narrow to; omit for the whole garden"` ObjectID *int64 `json:"objectId" description:"optional bed to narrow to; omit for the whole garden"`
From string `json:"from" description:"optional earliest observed date, YYYY-MM-DD"` From string `json:"from" description:"optional earliest observed date, YYYY-MM-DD"`
To string `json:"to" description:"optional latest observed date, YYYY-MM-DD"` To string `json:"to" description:"optional latest observed date, YYYY-MM-DD"`
Offset int `json:"offset" description:"how many entries to skip; pass the count you've already seen to page when hasMore is true"`
}) (any, error) { }) (any, error) {
q := service.JournalQuery{ObjectID: args.ObjectID, Limit: 50} q := service.JournalQuery{ObjectID: args.ObjectID, Limit: 50, Offset: args.Offset}
gitea-actions commented 2026-07-23 01:34:12 +00:00
Outdated
Review
Copy Link
Copy Source

🟠 read_journal exposes hasMore without offset parameter, making pagination impossible

correctness, maintainability · flagged by 3 models

  • internal/agent/tools.go:222 — The read_journal tool hardcodes Limit: 50 and returns hasMore to the LLM, but does not expose the offset parameter that the underlying service.JournalQuery supports. Because the tool adapter omits offset, an LLM that sees hasMore: true has no way to request the next page, making the pagination signal functionally useless. The tool should either accept an offset argument so the LLM can page, or not return hasMore at all.

🪰 Gadfly · advisory

🟠 **read_journal exposes hasMore without offset parameter, making pagination impossible** _correctness, maintainability · flagged by 3 models_ - `internal/agent/tools.go:222` — The `read_journal` tool hardcodes `Limit: 50` and returns `hasMore` to the LLM, but does not expose the `offset` parameter that the underlying `service.JournalQuery` supports. Because the tool adapter omits `offset`, an LLM that sees `hasMore: true` has no way to request the next page, making the pagination signal functionally useless. The tool should either accept an `offset` argument so the LLM can page, or not return `hasMore` at all. <sub>🪰 Gadfly · advisory</sub>
if args.From != "" { if args.From != "" {
q.From = &args.From q.From = &args.From
} }
1
 @@ -230,6 +230,7 @@ func (a *adapter) readJournal(ctx context.Context, args struct {
if err != nil { if err != nil {
return nil, err return nil, err
} }
// hasMore is actionable now: re-call with offset += len(entries) to page.
return map[string]any{"entries": entries, "hasMore": hasMore}, nil return map[string]any{"entries": entries, "hasMore": hasMore}, nil
} }
1
 @@ -261,11 +262,9 @@ func (a *adapter) removePlanting(ctx context.Context, args struct {
PlantingID int64 `json:"plantingId" description:"plop to remove (its id from describe_garden)"` PlantingID int64 `json:"plantingId" description:"plop to remove (its id from describe_garden)"`
Version int64 `json:"version" description:"the plop's current version (from describe_garden)"` Version int64 `json:"version" description:"the plop's current version (from describe_garden)"`
}) (any, error) { }) (any, error) {
// Soft-remove: set removed_at to today, mirroring clear_object, so the plop // Soft-remove via the service, so removed_at is stamped from the same
// stays in the planting history and the change is undoable. // (injectable) clock clear_object uses rather than the adapter's wall clock.
gitea-actions commented 2026-07-23 01:34:12 +00:00
Outdated
Review
Copy Link
Copy Source

🟠 remove_planting stamps removed_at from wall-clock time.Now() instead of the service's injectable s.now(), diverging from the clear_object path it claims to mirror

correctness, error-handling, maintainability · flagged by 3 models

  • internal/agent/tools.go:266 — removePlanting computes today := time.Now().UTC().Format("2006-01-02") from the wall clock, but every other date-stamping path in the service uses the injectable clock (s.now().UTC().Format(dateLayout)): CreatePlanting at internal/service/plantings.go:104, ClearObjectPlantings's soft-remove path at internal/service/ops.go:413, and the fill path at internal/service/ops.go:238. now is an injectable func() time.Time on Service (`internal/s…

🪰 Gadfly · advisory

🟠 **remove_planting stamps removed_at from wall-clock time.Now() instead of the service's injectable s.now(), diverging from the clear_object path it claims to mirror** _correctness, error-handling, maintainability · flagged by 3 models_ - `internal/agent/tools.go:266` — `removePlanting` computes `today := time.Now().UTC().Format("2006-01-02")` from the **wall clock**, but every other date-stamping path in the service uses the injectable clock (`s.now().UTC().Format(dateLayout)`): `CreatePlanting` at `internal/service/plantings.go:104`, `ClearObjectPlantings`'s soft-remove path at `internal/service/ops.go:413`, and the fill path at `internal/service/ops.go:238`. `now` is an injectable `func() time.Time` on `Service` (`internal/s… <sub>🪰 Gadfly · advisory</sub>
today := time.Now().UTC().Format("2006-01-02") return a.svc.RemovePlanting(ctx, a.actor, args.PlantingID, args.Version)
return a.svc.UpdatePlanting(ctx, a.actor, args.PlantingID,
service.PlantingPatch{SetRemovedAt: true, RemovedAt: &today}, args.Version)
} }
func (a *adapter) listSeedLots(ctx context.Context, args struct { func (a *adapter) listSeedLots(ctx context.Context, args struct {
1
internal/service/plantings.go
+11
View File
@@ -178,6 +178,17 @@ func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64,
return updated, nil return updated, nil
} }
// RemovePlanting soft-removes a single plop — the one-plop counterpart to
// ClearObject, used by the agent's remove_planting tool. It stamps removed_at
// from the service clock (s.now()), same as ClearObject and the fill path, so the
// removal date can't diverge by which caller set it; then delegates to
// UpdatePlanting for the editor-role check, version guard and history record.
func (s *Service) RemovePlanting(ctx context.Context, actorID, plantingID, version int64) (*domain.Planting, error) {
today := s.now().UTC().Format(dateLayout)
return s.UpdatePlanting(ctx, actorID, plantingID,
PlantingPatch{SetRemovedAt: true, RemovedAt: &today}, version)
}
// plantingEditSummary describes a plop edit for the history list. Soft-removal // plantingEditSummary describes a plop edit for the history list. Soft-removal
// ("clear bed", harvested) is the one edit worth naming specifically — it reads // ("clear bed", harvested) is the one edit worth naming specifically — it reads
// as a removal to the person who did it, not as an edit. // as a removal to the person who did it, not as an edit.