feat!: refuse derived-field writes and verify a TeamIteration clear

[man8-octoflow]

Summary

TargetProcess answers some writes with a success status that does not mean the
write took effect. This adds a guard for two such fields, and the two guards are
deliberately opposite shapes, because the two failures are knowable at different
moments.

A work item's effort roll-ups cannot be checked after the fact, so they are
refused before sending. Effort, EffortCompleted and EffortToDo are each
the sum of the corresponding field over the item's RoleEfforts. A direct write
to one is answered 200 whether TP stored the value or recomputed it from those
rows and changed nothing, and no re-read separates "stored because nothing
overrode it" from "recomputed back to the same number". The four write methods
therefore raise ValueError before any request and name the RoleEffort route,
which is deterministic whatever the collection holds.

A TeamIteration clear can only be known after the write, so it is verified
after it. TP cascades a parent's team iteration onto its children, so a child's
explicit null is answered 200 whether the field cleared, was discarded, or
cleared and was immediately re-acquired from the parent — and an item that should
be unscheduled silently stays scheduled. clear_team_iteration sends the clear
and checks it.

Neither guard reuses the other's path.

No GitHub issue; this change is tracked outside this repository.

Changes

Five commits.

1. feat!: refuse a direct write to the derived effort roll-ups

• New resources/_derived.py declares the set once as
  ASSIGNABLE_DERIVED_FIELDS and holds the refusal (check_derived_fields, and
  per batch item check_derived_items). Its module docstring carries the design
  record: why before the request, and why ValueError rather than
  ReadOnlyViolation.
• BaseResource.derived_fields applies it on create, update, create_many
  and update_many; AssignableResource carries the declaration for the six
  work-item managers.
• entities mirrors it for every spelling of those six collections and of the
  untyped Assignable-derived ones, so the generic accessor is not the way round
  the typed guard. A walk test fails if a declaring manager is unreachable
  through that map — the same shape as the existing ignored-filter walk.
• allow_derived=True sends the write anyway. See The escape hatch is the
  point, not a loophole below.
• The AssignableEntity and RoleEffortsResource docstrings now say where a
  work item's effort actually lives, pinned by a test so neither can be silently
  reverted.

2. feat: verify that a TeamIteration clear actually cleared

• clear_team_iteration(id) on the six work-item managers. It is
  update(id, TeamIteration=None, verify=True) and nothing more — the same
  write, the same single narrowed re-read, the same comparison — with the failure
  re-raised as the new TeamIterationCascadeError.
• TeamIterationCascadeError subclasses VerificationError, so a caller already
  handling "the re-read did not show the write" catches it; the distinct type is
  for the caller that treats the cascade differently, because the remedy is not a
  retry but clearing or detaching the parent.
• No verify=False, deliberately: an unverified clear cannot be told from a
  failed one, which is the method's whole reason for existing. A caller wanting
  the bare write calls update directly.

3. docs: note the escape hatch the recorded write fixtures take

docs/testing.md's write-path recording rules describe Effort as the numeric
field each update sends and reads back. It is now a refused roll-up, so that
passage says the sends pass allow_derived=True, and why they may.

4. docs: split observability and time logging into their own files

Two tracked files were at the 1000-line ceiling the pre-commit and CI gate
enforces — SPEC.md at 999, and docs/USAGE.md at 992 once the two guards were
documented — so neither could take another paragraph. SPEC.md's Observability
section moves to docs/observability.md and docs/USAGE.md's Logging time to
docs/time.md, both following the precedent SPEC.md already sets for its
Testing Approach section: content unchanged, a stub in its place naming the new
file. See Nothing was lost in the extractions below.

5. docs: state both write contracts in the specification

### Derived-field writes next to the silently-ignored queries it shares its
posture with, ### Unscheduling: the TeamIteration cascade after the verified
writes it is built from, plus TeamIterationCascadeError in the exceptions list
and allow_derived in the resource signatures.

The escape hatch is the point, not a loophole

An unconditional refusal would have been wrong, and this repository's own
recorded fixtures are what say so. test_verified_update_returns_the_re_read
sends Effort on a sandbox story with verify=True and asserts it on the
independent re-read, with entity_version is None proving it is the re-read
and not the echo. So a direct write to a roll-up demonstrably can land.

That is exactly why the guard refuses rather than warns: the same request against
an item that carries RoleEffort rows is answered with the same success status
and a different outcome, and the caller cannot tell which from the response. The
escape hatch exists for the caller who knows which case they are in. The recorded
write suite is that caller — each entity it writes is created in the same run —
and its four sends now pass allow_derived=True with the reasoning at the call
site. No cassette was re-recorded: the keyword is consumed library-side, so
every request body is byte-identical and the recordings replay untouched.

The unit verified-write tests took the other option and moved their probe field
from Effort to NumericPriority, so the library's own unit tests do not opt out
of the library's own guard.

The specification states only what those recordings evidence — that such a write
can land, that it can equally be recomputed away, and that the response does not
distinguish them — and attributes the zero-RoleEffort mechanism as the likely
explanation rather than established behaviour. Asserting an unproven mechanism
would be the very failure these two guards exist to prevent.

Nothing was lost in the extractions

Each moved block round-trips byte for byte against the revision it came from,
with only the heading level shifted for a standalone file and one provenance
sentence added — checked by rebuilding the original block from the new file and
comparing (87 lines and 104 lines, both identical).

The one cross-reference this breaks — docs/USAGE.md's own link to the
custom-activity subsection — now points into docs/time.md. No other file
referenced either section's anchors, and the six __all__ names the
observability text mentions appear in docs/observability.md with the same
occurrence counts they had in SPEC.md.

SPEC.md is now 981 lines and docs/USAGE.md 892, leaving 19 and 108 of
headroom. resources/base.py is 989, leaving 11 — the per-item bulk loop lives
in _derived.py partly to keep it there.

Testing

Run against the head commit, each as its own command:

Gate	Result
uv run ruff check .	All checks passed
uv run ruff format --check .	170 files already formatted
uv run mypy --strict src	no issues in 56 source files
uv run pytest -q --cov	1505 passed, TOTAL 99%
uv run pre-commit run --all-files --hook-stage pre-commit	all hooks passed
uv run pre-commit run --all-files --hook-stage pre-push	all hooks passed
uv run python scripts/check_large_files.py	exit 0
uv run python scripts/check_internal_refs.py	exit 0
uv run python scripts/check_todos.py	exit 0
uv run python scripts/check_model_coverage.py	exit 0

Every file this touches is at 100% line and branch coverage
(_derived.py, assignables.py, exceptions.py, role_efforts.py).

55 tests added. The suite went from 1450 to 1505 with no test removed.

What the new tests actually pin

• The refusal never reaches the wire. Every refusal test runs a READWRITE
  client over an httpx.MockTransport that raises if reached, and asserts the
  request log is empty. READWRITE deliberately — a READONLY client would raise
  ReadOnlyViolation first and prove nothing about this guard.
• The clear is the shared verified-write path, not a second mechanism. One
  test asserts clear_team_iteration's request sequence — method, path, every
  query parameter, and body bytes — is identical to
  update(..., TeamIteration=None, verify=True)'s. A bespoke write-then-read
  inside the method would look the same from outside until the verified-write
  rules changed underneath it; this fails if they diverge.
• The echo is never mistaken for the re-read. In the landing test the mock's
  write echo still carries the iteration, and in the failing test the echo says
  the clear worked — so returning the echo fails in one direction and misses the
  cascade in the other.
• The boundary cannot widen silently. A test asserts Progress, TimeSpent
  and TimeRemain are not guarded and do reach the wire.
• tests/test_exceptions.py's existing walk over every library error caught the
  new exception's absence from its pickle table, which is a fair signal about the
  suite.

Honest note on mutation testing

I did not construct a meaningful mutation test. Against the true base revision
neither facility exists, so every new test fails with an AttributeError or a
missing refusal — that establishes the tests exercise new code and nothing more.
What they establish beyond that is the identity assertion and the empty-log
assertions above, both of which fail against a plausible wrong implementation.
Stating it plainly rather than dressing it up.

Context

• Breaking, and the CHANGELOG carries the migration. A create or update
  naming one of the three roll-ups now raises ValueError before sending. Write
  the role's own row through client.role_efforts — the error message carries
  the query that finds it — or pass allow_derived=True. RoleEffort's own
  effort fields are stored as written and are unaffected.
• allow_derived shadows a wire field literally named AllowDerived on
  create and update, the same pre-existing hazard verify has on update.
  No such TargetProcess field is known.
• Only the declared set is refused. A work item carries other computed
  numbers (Progress, TimeSpent, TimeRemain, LeadTime, CycleTime) and
  none is declared: each would need its own route named in its own message and
  the same live evidence an ignored-filter entry is added on. Widening a guard
  without evidence is the same error as asserting an unproven mechanism.
• Follow-ups, none of them reasons to grow this diff. clear_team_iteration
  is single-entity, so a caller unscheduling a batch drops to
  update_many(..., verify=True) and gets a bare VerificationError rather than
  the cascade error. The generic entities accessor carries no work-item
  behaviour at all — no clear_team_iteration and no advance_state — which is
  consistent with what was already there. A recorded live cassette for the
  cascade is still outstanding; it needs a parent/child pair built on the sandbox
  first, and is tracked outside this repository.
• SPEC.md's Public API Surface and the exceptions it defines are back in sync:
  the new exception is listed, and the surface's remaining gaps are unchanged
  from the base revision (verified by running the same check against both).

Checklist

• uv run ruff check . and uv run ruff format --check . pass
• uv run mypy --strict src passes
• uv run pytest -q passes and coverage stays at or above 90% — 99%
• uv run pre-commit run --all-files --hook-stage pre-commit passes
• uv run pre-commit run --all-files --hook-stage pre-push passes
• New public functions, methods, and classes have docstrings
• No real credentials, tokens, or PII are added to the diff
• [~] TODO/FIXME/HACK/XXX markers name an issue — none added

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: man8/targetprocess-py/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 14a6f53c-74ca-4039-99f0-2e03f239b404

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}