Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/design/engine.md`:
- Around line 289-290: Update the post-execution lifecycle docs to describe the
newly exposed enum value PARKED: state explicitly whether a PARKED termination
is a no-op that leaves the task in its current state (same behavior as MAX_TURNS
and BUDGET_EXHAUSTED) or whether it transitions to a specific state;
specifically modify the post-execution section in docs/design/engine.md to
mention PARKED alongside MAX_TURNS and BUDGET_EXHAUSTED and state the exact
semantics (e.g., "PARKED: leaves task in current state" or the alternative) and
keep the note about max_turns defaulting to 20 intact for context.

In `@src/ai_company/engine/agent_engine.py`:
- Around line 376-393: The code unconditionally logs
EXECUTION_ENGINE_TASK_TRANSITION and calls sync_to_task_engine after calling
_apply_recovery even when recovery returned the original ExecutionResult; update
the logic in the recovery handling (use the local variable execution_result and
its .context.task_execution) to compare the pre-recovery status and
post-recovery status (e.g., capture pre_status =
execution_result.context.task_execution.status before calling _apply_recovery
and post_status after) and only emit the EXECUTION_ENGINE_TASK_TRANSITION log
and call sync_to_task_engine when post_status != pre_status; extract this
compare-and-sync behavior into a small helper (used by both the current block
around execution_result handling and the other recovery block that also
logs/syncs) so the two sites share the same implementation and cannot drift,
referencing _apply_recovery, ExecutionResult, ctx.task_execution,
EXECUTION_ENGINE_TASK_TRANSITION, and sync_to_task_engine.

In `@src/ai_company/engine/task_sync.py`:
- Around line 32-110: The sync_to_task_engine function exceeds the 50-line
limit; split its non-success paths into two private helpers: create a helper
_handle_sync_exception(exc, agent_id, task_id, target_status, critical) to
contain the except Exception branch (determine the error message for
TaskEngineError vs other exceptions and call logger.error or logger.warning
based on critical), and another helper _handle_rejected_mutation(result,
agent_id, task_id, target_status, critical) to contain the else:
rejected-mutation logging (log error text, error_code and use correct severity).
Refactor sync_to_task_engine to keep only the happy-path mutation
construction/await and then call these two helpers from the except and rejected
branches, preserving existing args and log keys (EXECUTION_ENGINE_SYNC_FAILED,
EXECUTION_ENGINE_TASK_SYNCED) and propagating MemoryError/RecursionError
unchanged.
- Around line 170-182: The call to _transition_to_complete can partially succeed
(e.g., move IN_PROGRESS -> IN_REVIEW) then raise, but the except block currently
returns the original ctx, losing the furthest-synced state; update the except
handler so that on exception you try to re-load the latest task context from the
engine (using task_engine and agent_id/task_id) and set ctx to that latest
context (falling back to the original ctx only if reload fails), then return
execution_result.model_copy(update={"context": ctx}); keep references to
_transition_to_complete, task_engine, execution_result, ctx, agent_id, and
task_id to locate the change.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit/engine/test_task_sync.py`:
- Around line 238-251: The test test_critical_flag_does_not_raise currently only
checks that sync_to_task_engine does not raise when TaskEngineError is raised
with critical=True; update the test to also assert that the expected log
event/level is emitted (e.g., capture logs or use a test logger fixture and
assert an ERROR/CRITICAL log entry was produced) so the behavior "changes log
level" is actually verified; reference sync_to_task_engine, TaskEngineError and
the critical=True invocation to locate where to add the log capture and
assertion.
- Around line 439-481: Replace the two almost-identical tests
test_max_turns_returns_unchanged and test_budget_exhausted_returns_unchanged
with a single parametrized test using pytest.mark.parametrize over the
termination reasons (TerminationReason.MAX_TURNS and
TerminationReason.BUDGET_EXHAUSTED); inside the new test reuse
AgentContext.from_identity, ctx.with_task_transition,
_make_execution_result(reason=reason), and call apply_post_execution_transitions
with the same agent_id/task_id/task_engine arguments, asserting the returned
value is the same object (assert out is result); keep the original test name(s)
descriptive and remove the duplicated functions.
- Around line 180-237: Replace the four near-duplicate tests
(test_task_engine_error_swallowed, test_unexpected_exception_swallowed,
test_memory_error_propagates, test_recursion_error_propagates) with a single
parametrized test using pytest.mark.parametrize that iterates over cases built
with _make_mock_task_engine(..., side_effect=...) and an indicator of whether
the exception should propagate; inside the test call sync_to_task_engine(...)
for each case and use pytest.raises(expected_exception) for the
MemoryError/RecursionError cases and a simple await (no raise) for the swallowed
cases (TaskEngineError and RuntimeError); reference sync_to_task_engine and
_make_mock_task_engine in the new parametrized test to locate and reuse the same
setup.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ai_company/engine/task_sync.py`:
- Line 76: The current code creates a `log` alias (e.g., `log = logger.error if
critical else logger.warning`) which violates the naming rule; remove any `log =
...` assignments and instead call `logger.error(...)` or `logger.warning(...)`
directly based on the `critical` condition at each call site. Locate the `log =
...` assignment and every subsequent `log(...)` usage (notably the blocks around
the comment ranges 76, 94-101, and 114-121) and replace them with conditional
calls like `if critical: logger.error(...) else: logger.warning(...)` so
`logger` remains the only logger variable name. Ensure no new logger aliases
(`log`, `_logger`, etc.) are introduced elsewhere in the function or module.
- Around line 150-205: The function apply_post_execution_transitions is over 50
lines and should be split into smaller helpers: extract a shutdown handler
(e.g., handle_shutdown) that invokes _transition_to_interrupted with the same
args and returns the resulting ExecutionResult or early return when
ctx.task_execution is None; extract a completed handler (e.g.,
handle_completed_transitions) that receives the current ctx, agent_id, task_id,
task_engine and runs the for loop over _COMPLETION_STEPS calling
_transition_and_sync and logging/exiting on ValueError/ExecutionStateError
exactly as now; keep the final merge/return in apply_post_execution_transitions
to call the new helpers and return
execution_result.model_copy(update={"context": ctx}) while preserving
propagation of MemoryError and RecursionError and all existing logging and error
strings (EXECUTION_ENGINE_ERROR, step_reason, _transition_to_interrupted,
_transition_and_sync).

---

Outside diff comments:
In `@src/ai_company/engine/agent_engine.py`:
- Around line 344-420: Split _post_execution_pipeline into small helpers: move
cost recording into a helper (e.g., _record_execution_costs) that calls
record_execution_costs with the same args and tracker, move transition logic
into an _apply_post_transitions helper that invokes
apply_post_execution_transitions, extract the recovery+sync block into
_handle_recovery_and_sync which captures pre_recovery_status, calls
self._apply_recovery, logs EXECUTION_ENGINE_TASK_TRANSITION, and calls
sync_to_task_engine exactly as before, and move the classification block into
_classify_execution_errors which wraps classify_execution_errors in the same
try/except (re-raising MemoryError and RecursionError, catching Exception and
logging EXECUTION_ENGINE_ERROR with exc_info). Then have
_post_execution_pipeline call these helpers in the same order (record ->
transitions -> if error then handle_recovery_and_sync -> classify) and return
the final execution_result unchanged; ensure all original symbols
(record_execution_costs, apply_post_execution_transitions, self._apply_recovery,
sync_to_task_engine, classify_execution_errors, self._error_taxonomy_config,
EXECUTION_ENGINE_TASK_TRANSITION, EXECUTION_ENGINE_ERROR, TerminationReason) and
behavior (pre_recovery_status capture and exact log messages/parameters) are
preserved.

---

Duplicate comments:
In `@src/ai_company/engine/agent_engine.py`:
- Around line 385-399: The transition logging/sync block should only run when
the task status actually changed: add a check comparing pre_recovery_status (or
pre_*_status) against ctx.task_execution.status (or post status) and skip
logging/sync if equal; extract the duplicated logic into a single helper (e.g.,
_emit_task_transition_and_sync or similar) that accepts agent_id, task_id,
pre_status, post_status and the task_engine, calls
logger.info(EXECUTION_ENGINE_TASK_TRANSITION, ...) and then awaits
sync_to_task_engine(...) so both call sites (the blocks using ctx.task_execution
and the other site referenced around the second occurrence) call this helper
only when pre_status != post_status.

In `@src/ai_company/engine/task_sync.py`:
- Around line 43-122: sync_to_task_engine is too long; split its
exception-handling and rejected-mutation branches into small private helpers to
keep the function under 50 lines. Create e.g. _handle_sync_exception(exc,
agent_id, task_id, target_status, critical) that re-raises
MemoryError/RecursionError/asyncio.CancelledError and otherwise logs using
logger.error or logger.warning based on the critical flag (preserving the
EXECUTION_ENGINE_SYNC_FAILED message and exc_info). Create e.g.
_handle_rejected_mutation(result, agent_id, task_id, target_status, critical)
that logs the rejection (preserving result.error/result.error_code and
EXECUTION_ENGINE_SYNC_FAILED) and keep sync_to_task_engine focused on building
the mutation, awaiting task_engine.submit, calling the two helpers from the
except/else branches, and returning on success (preserve the debug log for
EXECUTION_ENGINE_TASK_SYNCED and unchanged semantics).

In `@tests/unit/engine/test_task_sync.py`:
- Around line 238-251: The test test_critical_flag_does_not_raise currently only
ensures no exception when calling sync_to_task_engine with critical=True but
does not assert that logging severity changed; update the test to capture/log
records (e.g., using caplog or a mock logger) around the call and assert that a
log entry at the expected higher severity (e.g., ERROR/CRITICAL) was emitted by
sync_to_task_engine when the TaskEngineError("unavailable") side_effect occurs
on mock_te; reference the test function test_critical_flag_does_not_raise, the
mock task engine mock_te, and the sync_to_task_engine invocation to locate where
to add the log capture and severity assertion.
- Around line 180-237: Collapse the four nearly identical tests
(test_task_engine_error_swallowed, test_unexpected_exception_swallowed,
test_memory_error_propagates, test_recursion_error_propagates) into one
parametrized pytest using `@pytest.mark.parametrize`: enumerate cases of
(side_effect_exception, should_raise) where side_effects are
TaskEngineError("engine down"), RuntimeError("connection lost"),
MemoryError("out of memory"), RecursionError("maximum recursion depth exceeded")
and should_raise is False for the first two and True for the latter two; within
the single test, call _make_mock_task_engine(side_effect=...) to create mock_te
and invoke sync_to_task_engine(...) and assert either that the call completes
(when should_raise is False) or that it raises the expected exception with
pytest.raises(..., match=...) when should_raise is True, preserving test names
and docstrings as needed for clarity.
- Around line 456-498: Two tests (test_max_turns_returns_unchanged and
test_budget_exhausted_returns_unchanged) duplicate the same logic; refactor them
into a single parametrized test using pytest.mark.parametrize over
TerminationReason values (TerminationReason.MAX_TURNS,
TerminationReason.BUDGET_EXHAUSTED) to avoid duplication. Update the test
function to accept a termination_reason parameter, reuse
AgentContext.from_identity, ctx.with_task_transition, and
_make_execution_result(…, reason=termination_reason), then call
apply_post_execution_transitions and assert the result is unchanged; keep
existing fixtures (sample_agent_with_personality, sample_task_with_criteria) and
the same agent_id/task_id arguments.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/unit/engine/test_task_sync.py`:
- Around line 489-525: Replace the manual monkeypatching of
AgentContext.with_task_transition (using original_method and assigning
patched_transition) with unittest.mock.patch.object to ensure automatic
restoration; specifically, patch AgentContext.with_task_transition inside a with
patch.object(AgentContext, "with_task_transition", new=patched_transition):
block (or as a decorator) around the invocation of
apply_post_execution_transitions, keeping the same patched_transition behavior
that raises ExecutionStateError on the second call and preserving references to
call_count and original_transition so assertions about
out.context.task_execution and TaskStatus.IN_REVIEW remain valid.

---

Duplicate comments:
In `@src/ai_company/engine/task_sync.py`:
- Around line 171-227: The function apply_post_execution_transitions is
approaching the 50-line limit; to keep it maintainable and avoid exceeding the
threshold, extract the COMPLETED-path loop that iterates over _COMPLETION_STEPS
and calls _transition_and_sync into a private helper (for example
_apply_completion_steps) and have apply_post_execution_transitions call that
helper, preserving the existing SHUTDOWN delegation to
_transition_to_interrupted and identical error handling/logging semantics.