docs(schedule): record the deliberate ExecutionResult dataclass upgrade
Review (2026-08-21) found that stage 1 promoted ExecutionResult from a plain class to @dataclass(frozen=True) along the way. No caller mutates or compares these objects by identity, so the only externally visible change is structured log output. User opted to keep the upgrade. - domain/execution.py module docstring: explicit note that the frozen + value-equality form is a deliberate enhancement, not a behavioral accident - CLAUDE.md "Schedule service layering" lesson: add a "don't silently upgrade dataclass-ness during a structural-only refactor" note so future refactors copy class definitions verbatim unless they intend to tighten semantics explicitly No code change; tests still 29 green. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -84,6 +84,7 @@ Lessons from the zero-behavior-change refactor that split the flat 11-file `sche
|
||||
- **Mock `patch()` string targets and in-function lazy imports are invisible to import-line greps.** `test_janitor.py` patched `"schedule.orchestrator.session_scope"` and `test_worker.py` patched `"schedule.service.build_object_store"`; `worker.py` also had a `from schedule.service import ...` *inside a function body*. When files move, these silently no-op against the old path — and hard-crash once the old file is deleted. After any move, grep the whole tree for the old module path (including tests) and rewrite every hit, not just top-level imports.
|
||||
- **A new package dir shadows a same-named flat module.** Creating `schedule/execution/` makes the old `schedule/execution.py` silently dead code (the package wins import resolution), so move-then-delete, don't just copy. `git` usually detects these as renames, which keeps the diff reviewable.
|
||||
- **Docstring references survive file deletion.** After removing flat files, `:class:\`schedule.worker.NodeExecutor\``-style text can linger in docstrings and render as broken links. Grep for the old module name one more time at cleanup and rewrite comment-only refs too.
|
||||
- **Don't silently upgrade dataclass-ness during a "structural only" refactor.** Stage 1 moved `ExecutionResult` from the flat `schedule/execution.py` into `domain/execution.py` and *decorated* it with `@dataclass(frozen=True)` along the way. Pre-refactor it was a plain class. Review (2026-08-21) caught that this changes three things at once: identity-`==` becomes value-`==`, mutation raises `FrozenInstanceError`, and `repr()` becomes structured. No caller in the repo mutates or compares these objects, so the only externally visible change is log format — but it is *not* "zero behavior change." If you want strict behavioral equivalence during a structural move, copy the class definition verbatim and document any intentional semantic tightening.
|
||||
|
||||
### Frontend state + routing (zustand + React Router v8)
|
||||
|
||||
|
||||
@@ -2,6 +2,12 @@
|
||||
|
||||
Pure value objects — no I/O, no logging, no model imports. Safe to import
|
||||
from any layer.
|
||||
|
||||
Note: ``ExecutionResult`` is a frozen dataclass here, while the pre-refactor
|
||||
flat ``schedule/execution.py`` defined it as a plain class. The frozen +
|
||||
value-equality upgrade is a deliberate enhancement (review verification,
|
||||
2026-08-21): no caller mutates the object and no caller relies on identity
|
||||
comparison, so the only visible change is structured log output.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
Reference in New Issue
Block a user