[Kohei-Wada]

Summary

OptimizationSummaryBuilder ran a second repository.get_all() on every optimize run, re-deserializing every task with its tags and allocations, although OptimizeScheduleUseCase had already fetched the full list. Its ad-hoc filter also counted an archived task without a schedule as "unscheduled".

Changes

• Use case: strategies return copies of the tasks they schedule (allocation_helpers uses deepcopy, and round-robin and genetic copy too). The use case therefore builds the post-save state by overlaying result.tasks on the already-fetched all_tasks by ID. Tasks cleared under force_override were mutated in place, so they are already current. That list is passed to build().
• Builder: no longer takes a repository. _find_unscheduled_tasks(tasks_after) filters with Task.should_count_in_workload(), which excludes archived as well as finished tasks.

Correction to the issue's impact

The issue says archived tasks are "reported to the user" as unscheduled. That is not reachable today. The server's OptimizationResponse does not include summary.unscheduled_tasks, and the client rebuilds its unscheduled list from failures (optimization_converters.py). I confirmed this on a live server: with an archived, unscheduled task present, the client sees unscheduled: [] both before and after this change. The user-visible effect of this PR is the removed full-table read. The unscheduled_tasks filter fix corrects the core DTO for any future consumer.

Verification

• New use-case test: one schedulable task plus one archived, estimated, unscheduled task.
  • Asserts the summary's unscheduled_tasks == [] and that get_all is called exactly once.
  • It failed before the change: the archived task was listed.
• New builder test: an archived, unscheduled task is not reported. The existing builder tests now pass the post-save list explicitly.
• make check and make test pass.

Closes #1137

[amazon-q-developer]

Review Summary

This PR correctly eliminates the redundant repository.get_all() call in OptimizeScheduleUseCase by passing the already-fetched task list through to OptimizationSummaryBuilder, and fixes the filter in _find_unscheduled_tasks to properly exclude archived tasks via should_count_in_workload(). The overall approach is sound and the new use-case test correctly verifies both get_all.call_count == 1 and the archived-task exclusion.

Critical finding

One blocking issue was identified in optimization_summary_builder.py: the assert statement used to guard task.id is None before constructing TaskSummaryDto is stripped by the Python optimizer (-O / PYTHONOPTIMIZE), making it a no-op in production builds. This lets a None id silently flow into the DTO. A ValueError (or similar unconditional guard) should be used instead. See the inline comment for the suggested fix.

Minor observations (non-blocking)

• test_build_with_rescheduled_tasks in the builder test passes a raw string "2025-10-14 09:00:00" for task_states_before where the declared type is dict[int, datetime | None]. The test passes today because the check is is None, but it violates the contract and could mask future regressions. This is a pre-existing issue not introduced by this PR.
• The tasks_after overlay in the use case (saved_by_id.get(t.id, t) for t in all_tasks) is correct: strategies return copies of scheduled tasks, and tasks cleared under force_override are mutated in-place on the same all_tasks object references, so both code paths are reflected accurately without an extra DB round-trip.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[amazon-q-developer]

🛑 Crash Risk: assert statements are silently removed when Python is run with the -O (optimize) flag or PYTHONOPTIMIZE environment variable, making this invariant check dead code in optimized production builds. The subsequent TaskSummaryDto(id=task.id, ...) call will silently receive None as the id, propagating corrupt data into the optimization summary. Replace the assert with an unconditional ValueError raise to ensure the check is always active regardless of interpreter flags.