Skip to content

Commit 0f54477

Browse files
committed
style(agentex): collapse verbose comments/docstrings to one line
Self-review cleanup: tighten the multi-line comments and docstrings added for current_state down to single lines (drop AI-slop narration; keep only the non-obvious why), per the repo's comment-discipline convention.
1 parent 8e0fda6 commit 0f54477

11 files changed

Lines changed: 37 additions & 101 deletions

File tree

agentex/database/migrations/alembic/versions/2026_07_22_1200_add_task_current_state_b2c3d4e5f6a7.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818

1919

2020
def upgrade() -> None:
21-
# Opaque label mirroring an agent's StateMachine current state. Nullable and
22-
# additive; agents opt in by emitting it. Metadata-only add, non-blocking.
23-
# Idempotent (IF NOT EXISTS) so re-running on an environment that already has
24-
# the column is a no-op. Width matches TaskORM.current_state (String(255)).
21+
# Nullable additive column; idempotent, metadata-only, non-blocking.
2522
op.execute(
2623
"ALTER TABLE tasks ADD COLUMN IF NOT EXISTS current_state VARCHAR(255)"
2724
)

agentex/src/adapters/orm.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,7 @@ class TaskORM(BaseORM):
7575
cleaned_at = Column(DateTime(timezone=True), nullable=True)
7676
params = Column(JSONB, nullable=True)
7777
task_metadata = Column(JSONB, nullable=True)
78-
# Opaque, framework-agnostic label mirroring an agent's StateMachine current
79-
# state. Written best-effort by the agent on each transition; reconciled on
80-
# read. Orthogonal to `status` (Temporal workflow lifecycle). Bounded: a
81-
# state label is short, and the value is echoed onto every task_updated SSE
82-
# payload, so we cap it to avoid unbounded stream amplification.
78+
# Opaque agent-state label, orthogonal to `status`; capped since it rides every task_updated SSE payload.
8379
current_state = Column(String(255), nullable=True)
8480
# Many-to-Many relationship with agents
8581
agents = relationship("AgentORM", secondary="task_agents", back_populates="tasks")

agentex/src/api/routes/tasks.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,7 @@ async def update_task(
196196
id=task_id,
197197
task_metadata=request.task_metadata,
198198
merge_params=request.merge_params,
199-
# Pass through only when the client actually sent the key, so an explicit
200-
# null clears the label while an omitted field leaves it untouched.
199+
# Forward only when the client sent the key: explicit null clears, omitted leaves unchanged.
201200
current_state=(
202201
request.current_state
203202
if "current_state" in request.model_fields_set
@@ -224,8 +223,7 @@ async def update_task_by_name(
224223
name=task_name,
225224
task_metadata=request.task_metadata,
226225
merge_params=request.merge_params,
227-
# Pass through only when the client actually sent the key, so an explicit
228-
# null clears the label while an omitted field leaves it untouched.
226+
# Forward only when the client sent the key: explicit null clears, omitted leaves unchanged.
229227
current_state=(
230228
request.current_state
231229
if "current_state" in request.model_fields_set

agentex/src/api/schemas/tasks.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
from src.api.schemas.agents import Agent
88
from src.utils.model_utils import BaseModel
99

10-
# Upper bound on the opaque `current_state` label. Kept in sync with the
11-
# `TaskORM.current_state` column width (String(255)); a state label is short and
12-
# the value rides every task_updated SSE payload, so it is capped.
10+
# Bound on the current_state label; mirrors the String(255) column width.
1311
CURRENT_STATE_MAX_LENGTH = 255
1412

1513

@@ -68,9 +66,7 @@ class Task(BaseModel):
6866
None,
6967
title="Task metadata",
7068
)
71-
# No max_length on this response field on purpose: input is already bounded
72-
# by UpdateTaskRequest + the String(255) column, and enforcing the bound on
73-
# the read path would turn a future column-widening into a 500 on every read.
69+
# No bound on the read path: enforcing it would 500 if the column is ever widened (writes are already bounded).
7470
current_state: str | None = Field(
7571
None,
7672
title=(

agentex/src/domain/repositories/task_repository.py

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -257,22 +257,11 @@ async def merge_params(self, task_id: str, patch: dict) -> TaskEntity | None:
257257
async def update_mutable_fields(
258258
self, task_id: str, fields: dict[str, Any]
259259
) -> TaskEntity | None:
260-
"""Atomically set only the given scalar columns on a single task row.
261-
262-
Unlike ``update`` (a whole-row ``session.merge`` of a possibly-stale
263-
entity, which rewrites every column and can clobber a concurrently
264-
changed ``status``/``params``), this issues a single
265-
``UPDATE ... SET <only these columns> WHERE id = :id RETURNING *``.
266-
Touching only the supplied columns means a concurrent status transition
267-
or param merge is never reverted. With a non-empty ``fields`` this returns
268-
the updated entity, or ``None`` if no task with ``task_id`` exists.
269-
270-
``fields`` values are applied verbatim, so passing ``current_state=None``
271-
clears the column (callers distinguish "clear" from "omit" upstream).
272-
273-
An empty ``fields`` is a defensive no-op that returns the current task
274-
(raising ``ItemDoesNotExist`` if absent); the sole caller never reaches it,
275-
as it gates the call behind a non-empty field set.
260+
"""Atomically ``UPDATE ... SET <only the given columns> WHERE id`` — column-scoped,
261+
so (unlike ``update``'s whole-row merge) it can't clobber a concurrently changed
262+
``status``/``params``. Values apply verbatim (``current_state=None`` clears). Returns
263+
the updated entity, or ``None`` if the task doesn't exist. Empty ``fields`` is an
264+
unreachable defensive no-op (the sole caller gates on a non-empty set).
276265
"""
277266
if not fields:
278267
return await self.get(id=task_id)

agentex/src/domain/services/task_service.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -245,14 +245,8 @@ async def update_task(self, task: TaskEntity) -> TaskEntity:
245245
async def update_mutable_fields(
246246
self, task_id: str, fields: dict[str, Any]
247247
) -> TaskEntity | None:
248-
"""Atomically update only the given scalar columns on a task, then
249-
publish a task_updated event.
250-
251-
Column-scoped (see ``TaskRepository.update_mutable_fields``) so it cannot
252-
clobber a concurrently changed ``status``/``params`` — unlike
253-
``update_task``'s whole-row merge, which the status-writing callers
254-
(delete/fail/forward) still rely on. Returns the updated entity, or
255-
``None`` if the task no longer exists.
248+
"""Column-scoped atomic update of the given columns, then publish task_updated.
249+
Returns the updated entity, or ``None`` if the task no longer exists.
256250
"""
257251
updated_task = await self.task_repository.update_mutable_fields(
258252
task_id, fields

agentex/src/domain/use_cases/tasks_use_case.py

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@
1212

1313

1414
class _Unset:
15-
"""Sentinel distinguishing "field omitted" from "field explicitly set to null"
16-
in a PATCH-style update, so an explicit null can clear a column while an
17-
omitted field is left untouched."""
15+
"""Sentinel: PATCH field omitted (untouched) vs. explicitly null (cleared)."""
1816

1917

2018
UNSET: Any = _Unset()
@@ -106,11 +104,8 @@ async def update_mutable_fields_on_task(
106104
merge_params: dict[str, Any] | None = None,
107105
current_state: str | None | _Unset = UNSET,
108106
) -> TaskEntity:
109-
"""Update mutable fields on a task entity. This is used by our API since not all fields should be mutable.
110-
111-
``current_state`` uses the UNSET sentinel so an explicit null clears the
112-
column while an omitted field leaves it untouched. ``task_metadata`` keeps
113-
its legacy semantics (``None`` means "not supplied").
107+
"""Update mutable fields on a task. ``current_state`` uses the UNSET sentinel
108+
(explicit null clears, omitted leaves it); ``task_metadata`` None means "not supplied".
114109
"""
115110

116111
if not id and not name:
@@ -134,21 +129,15 @@ async def update_mutable_fields_on_task(
134129
):
135130
return task_entity
136131

137-
# `merge_params` is a separate atomic JSONB shallow-merge so concurrent
138-
# callers don't overwrite each other's fields. Run it first so the
139-
# refreshed entity it returns becomes the value we return if no scalar
140-
# field updates follow.
132+
# Atomic JSONB shallow-merge; run first so its refreshed entity is the fallback return.
141133
if merge_params:
142134
merged = await self.task_service.merge_task_params(
143135
task_entity.id, merge_params
144136
)
145137
if merged is not None:
146138
task_entity = merged
147139

148-
# Apply task_metadata/current_state via a single column-scoped atomic
149-
# update — one task_updated publish, and (unlike a whole-row merge) no
150-
# risk of clobbering a concurrently changed status/params. current_state
151-
# rides that publish, powering reactive delivery to stream consumers.
140+
# Single column-scoped write → one task_updated publish, no whole-row clobber.
152141
fields: dict[str, Any] = {}
153142
if task_metadata is not None:
154143
fields["task_metadata"] = task_metadata
@@ -159,11 +148,7 @@ async def update_mutable_fields_on_task(
159148
task_entity.id, fields
160149
)
161150
if updated is None:
162-
# The row vanished between the initial read and the write. No live
163-
# hard-delete path reaches here today (delete is a soft status
164-
# update, already guarded above), so this is defensive — but raise
165-
# rather than return stale data, matching the not-found contract
166-
# above and the sibling terminal-transition methods.
151+
# Row vanished mid-flight (defensive; no live hard-delete path). Raise, don't return stale.
167152
raise ItemDoesNotExist(f"Task {id or name} not found")
168153
task_entity = updated
169154

agentex/tests/integration/api/tasks/test_tasks_api.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -686,8 +686,7 @@ async def test_update_task_endpoint_success(
686686
async def test_update_task_current_state(
687687
self, isolated_client, isolated_repositories
688688
):
689-
"""PUT /tasks/{id} writes current_state; explicit null clears it while an
690-
omitted field leaves it untouched; point-read is source of truth."""
689+
"""PUT current_state: explicit null clears, omitted leaves it, point-read reconciles."""
691690
agent_repo = isolated_repositories["agent_repository"]
692691
agent = AgentEntity(
693692
id=orm_id(),
@@ -743,8 +742,7 @@ async def test_update_task_current_state(
743742
async def test_update_task_current_state_and_metadata_together(
744743
self, isolated_client, isolated_repositories
745744
):
746-
"""current_state + task_metadata in one PUT both persist (single write),
747-
without clobbering status."""
745+
"""current_state + task_metadata in one PUT both persist without clobbering status."""
748746
agent_repo = isolated_repositories["agent_repository"]
749747
agent = AgentEntity(
750748
id=orm_id(),
@@ -807,8 +805,7 @@ async def test_update_task_current_state_by_name(
807805
async def test_update_task_current_state_empty_string(
808806
self, isolated_client, isolated_repositories
809807
):
810-
"""Empty string is a valid label distinct from null (guards against a
811-
future falsy check treating "" as "unset")."""
808+
"""Empty string is a valid label distinct from null (guards a falsy-check regression)."""
812809
agent_repo = isolated_repositories["agent_repository"]
813810
agent = AgentEntity(
814811
id=orm_id(),
@@ -865,9 +862,7 @@ async def test_update_task_current_state_too_long_rejected(
865862
async def test_update_task_request_ignores_unknown_fields(
866863
self, isolated_client, isolated_repositories
867864
):
868-
"""Guards the extra="ignore" assumption a newer SDK relies on: a payload
869-
carrying fields the server doesn't model must 200 (drop them), not 422.
870-
Breaks loudly if UpdateTaskRequest ever switches to extra="forbid"."""
865+
"""Unknown fields are ignored (200, not 422) — guards the extra="ignore" SDK-compat assumption."""
871866
agent_repo = isolated_repositories["agent_repository"]
872867
agent = AgentEntity(
873868
id=orm_id(),

agentex/tests/integration/test_task_stream.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ async def collect_stream_events():
233233
async def test_current_state_update_triggers_stream_event(
234234
self, test_agent_and_task, tasks_use_case, streams_use_case
235235
):
236-
"""current_state rides the existing task_updated event, so a client
237-
subscribed to the task stream is pushed the new state reactively."""
236+
"""current_state rides the existing task_updated event (reactive push to subscribers)."""
238237
_agent, task = test_agent_and_task
239238

240239
stream_events = []
@@ -260,23 +259,20 @@ async def collect_stream_events():
260259
pass
261260

262261
stream_task = asyncio.create_task(collect_stream_events())
263-
# Let the subscription establish before the update; the stream is
264-
# tail-only, so an event published before we subscribe would be missed.
262+
# Let the tail-only subscription establish before the update, or the event is missed.
265263
await asyncio.sleep(0.1)
266264

267265
updated_task = await tasks_use_case.update_mutable_fields_on_task(
268266
id=task.id, current_state="awaiting_input"
269267
)
270268

271-
# Wait until the collector sees task_updated (it breaks on it) rather
272-
# than guessing a fixed delay; the timeout is only a failure ceiling.
269+
# Wait for the collector to see task_updated (it breaks on it); timeout is only a ceiling.
273270
try:
274271
async with asyncio.timeout(5):
275272
await stream_task
276273
except (TimeoutError, asyncio.CancelledError):
277274
stream_task.cancel()
278-
# Re-await so the task's own cancellation cleanup runs and no
279-
# "Task exception was never retrieved" warning leaks at teardown.
275+
# Re-await so cancellation cleanup runs and no dangling-task warning leaks at teardown.
280276
await asyncio.gather(stream_task, return_exceptions=True)
281277

282278
task_updated_events = [

agentex/tests/unit/services/test_task_service.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -941,8 +941,7 @@ async def test_update_task_with_task_metadata_changes(
941941
async def test_update_task_current_state_publishes_stream_event(
942942
self, task_service, agent_repository, sample_agent, redis_stream_repository
943943
):
944-
"""update_task persists current_state and carries it on the published
945-
task_updated event, so subscribed clients see the new state."""
944+
"""update_task persists current_state and carries it on the task_updated event."""
946945
await create_or_get_agent(agent_repository, sample_agent)
947946
created_task = await task_service.create_task(
948947
agent=sample_agent, task_name="task-for-current-state"
@@ -967,8 +966,7 @@ async def test_update_task_current_state_publishes_stream_event(
967966
async def test_update_mutable_fields_persists_and_publishes(
968967
self, task_service, agent_repository, sample_agent, redis_stream_repository
969968
):
970-
"""update_mutable_fields writes only the given columns and publishes a
971-
task_updated event carrying them."""
969+
"""update_mutable_fields persists the given columns and publishes task_updated."""
972970
await create_or_get_agent(agent_repository, sample_agent)
973971
created_task = await task_service.create_task(
974972
agent=sample_agent, task_name="task-for-mutable-fields"
@@ -994,10 +992,9 @@ async def test_update_mutable_fields_persists_and_publishes(
994992
async def test_update_mutable_fields_leaves_status_untouched(
995993
self, task_service, agent_repository, sample_agent, redis_stream_repository
996994
):
997-
"""The primitive is column-scoped: writing current_state does not touch
998-
status. (The use-case-level clobber regression — a stale read racing a
999-
status transition — is guarded in test_tasks_use_case.py; this only pins
1000-
that the repository UPDATE sets no columns beyond those supplied.)"""
995+
"""The primitive is column-scoped: writing current_state leaves status untouched
996+
(the use-case stale-read clobber regression is guarded in test_tasks_use_case.py).
997+
"""
1001998
await create_or_get_agent(agent_repository, sample_agent)
1002999
created_task = await task_service.create_task(
10031000
agent=sample_agent, task_name="task-for-noclobber"

0 commit comments

Comments
 (0)