fix: generate all slot timestamps in optimizer asTimestamps

[rishabhvaish]

Summary

asTimestamps only returned 1 timestamp instead of one per slot. The loop for i := range len(res) - 1 evaluated to range 0 since len(res) was always 1, so it never executed. Timestamp offsets also weren't cumulative.

Rewrote to iterate over all dt entries and accumulate timestamps from the first slot's start time.

Test plan

• go test ./core/... -v passes
• Verify optimizer schedule produces correct timestamps for multi-slot plans

🤖 Generated with Claude Code

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The new cumulative logic in asTimestamps uses dt[0] twice (once in the initial ts and again as the first d in the loop), which means the second timestamp is offset by dt[0] from the first rather than starting from the first slot’s start; consider iterating over dt[1:] after initializing ts from dt[0] to match the described behavior.
• For the empty dt case, consider returning an empty slice instead of nil so callers that iterate or append to the result don’t need to treat the zero-length case specially.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new cumulative logic in `asTimestamps` uses `dt[0]` twice (once in the initial `ts` and again as the first `d` in the loop), which means the second timestamp is offset by `dt[0]` from the first rather than starting from the first slot’s start; consider iterating over `dt[1:]` after initializing `ts` from `dt[0]` to match the described behavior.
- For the empty `dt` case, consider returning an empty slice instead of `nil` so callers that iterate or append to the result don’t need to treat the zero-length case specially.

## Individual Comments

### Comment 1
<location path="core/site_optimizer.go" line_range="624-625" />
<code_context>
-	res = append(res, eoh.Add(-time.Duration(dt[0])*time.Second))
-	for i := range len(res) - 1 {
-		res = append(res, eoh.Add(time.Duration(dt[i+1])*time.Second))
+	ts := eoh.Add(-time.Duration(dt[0]) * time.Second)
+	for _, d := range dt {
+		res = append(res, ts)
+		ts = ts.Add(time.Duration(d) * time.Second)
</code_context>
<issue_to_address>
**issue (bug_risk):** The new loop mixes an initial absolute offset with cumulative deltas, which likely produces incorrect timestamps.

Previously, each timestamp was computed as an independent offset from `eoh` (`eoh.Add(-dt[0])`, then `eoh.Add(dt[i+1])`). The new code sets `ts = eoh - dt[0]` once, then appends `ts` for every element in `dt` while cumulatively adding `d`, so you effectively apply `dt[0]` twice and switch from absolute offsets to cumulative deltas.

If `dt` is meant to be absolute offsets, keep computing each timestamp directly from `eoh`, e.g.:

```go
for _, d := range dt {
    res = append(res, eoh.Add(-time.Duration(d)*time.Second))
}
```

If `dt` is meant to be deltas between timestamps, start the loop from `dt[1:]` (so `dt[0]` is only used to init `ts`) and confirm that this cumulative sequence matches the intended timeline.
</issue_to_address>

### Comment 2
<location path="core/site_optimizer.go" line_range="618-619" />
<code_context>
 }

 func asTimestamps(dt []int) []time.Time {
+	if len(dt) == 0 {
+		return nil
+	}
+
</code_context>
<issue_to_address>
**suggestion:** Returning nil for empty input changes the previous behavior and may affect callers expecting a non-nil empty slice.

Previously, when `len(dt) == 0`, this function returned a non-nil empty slice; now it returns `nil`. In Go this can change behavior (e.g., JSON output, equality checks, or callers that rely on a non-nil slice while expecting `len(asTimestamps(...)) == 0`). If there’s no deliberate need for `nil`, consider returning `[]time.Time{}` here to keep the existing contract.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[andig]

why do we need this?

[rishabhvaish]

Good point, removed it. The only caller always passes a non-empty slice so the guard was dead code.

[andig]

asTimestamps only returned 1 timestamp instead of one per slot. The loop for i := range len(res) - 1 evaluated to range 0 since len(res) was always 1, so it never executed.

If that's the case, the only required change is apparently

len(dt)

so why change everything else? Please make sure to also add a test.

[andig]

Closed in d2a8813