Create and edit child profiles (Feature 03, task 1) #51

2026-09-15T12:34:14-04:00

dev_agent commented

2026-09-15 12:34:14 -04:00

First task of Feature 03 (Children Registry and Admin Allowlist): the business layer between the future Feature 09 service gate and ChildrenDao.

Delivered

children.py: create_child (display name required + trimmed, whitespace-only rejected; duplicate display name ALLOWED but logged as a warning — case-insensitive, whitespace-normalised on both sides, per the done-condition), edit_child (_UNSET leave-alone sentinel, returns the updated record; rename-to-different-child warns, rename-to-self does not), set_child_active (the only removal path — children are deactivated, never deleted), list_children (sort_order then id).
Explicit None on optional fields raises a clear ValueError instead of silently no-oping: the DAO's update deliberately writes only provided fields and cannot express NULL, so a clear request must not masquerade as one.
tests/test_children_registry.py: 27 tests — typed round-trips, name validation matrix, duplicate-name warning on create and rename (and its absence on rename-to-self and distinct names), partial edits, refusal of unsupported clears, deactivation round-trip, ordering.

Verification

uv run pytest: 846 passed (27 new).

Closes task 90a3a7a3 (Feature 03).

First task of Feature 03 (Children Registry and Admin Allowlist): the business layer between the future Feature 09 service gate and ChildrenDao. ## Delivered - **children.py**: create_child (display name required + trimmed, whitespace-only rejected; duplicate display name ALLOWED but logged as a warning — case-insensitive, whitespace-normalised on both sides, per the done-condition), edit_child (_UNSET leave-alone sentinel, returns the updated record; rename-to-different-child warns, rename-to-self does not), set_child_active (the only removal path — children are deactivated, never deleted), list_children (sort_order then id). - Explicit None on optional fields raises a clear ValueError instead of silently no-oping: the DAO's update deliberately writes only provided fields and cannot express NULL, so a clear request must not masquerade as one. - **tests/test_children_registry.py**: 27 tests — typed round-trips, name validation matrix, duplicate-name warning on create and rename (and its absence on rename-to-self and distinct names), partial edits, refusal of unsupported clears, deactivation round-trip, ordering. ## Verification uv run pytest: 846 passed (27 new). Closes task 90a3a7a3 (Feature 03).

dev_agent added 1 commit 2026-09-15 12:34:14 -04:00

Child-profile business layer (Feature 03, task 1) 61b77cc2db

Business-layer functions over ChildrenDao for creating and editing
child profiles, so the Feature 09 service gate never touches the DAO
directly.

- children.py: create_child (display name required and trimmed,
  whitespace-only rejected; duplicate display name allowed but logged
  as a warning, case-insensitive and whitespace-normalised on both
  sides), edit_child (_UNSET sentinel for leave-alone; returns the
  updated record; renaming onto a different child's name warns, own
  name does not), set_child_active (the only removal path), and
  list_children (display order). Explicit None for optional fields
  raises instead of silently no-oping: the DAO's update deliberately
  writes only provided fields and cannot express NULL, so a clear
  request must not masquerade as one. UTC second-precision stamps per
  the timestamp policy.
- tests/test_children_registry.py: 27 tests covering the typed
  round-trips, name validation matrix, duplicate-name warning
  behaviour (create and rename, including the rename-to-self
  non-warning), partial edits, refusal of unsupported clears,
  deactivation round-trip and list ordering.

overseer added 1 commit 2026-09-15 14:08:26 -04:00

Review fix: duplicate-name scan excludes only the edited child 712527d5ba

Review round 1 (Codex gpt-5.6-terra) found: a case-only edit could
suppress the duplicate-name warning even when a DIFFERENT child
already carried the normalised name, because the edit path compared
only against the child's own old name. The scan now always runs on
the resulting state and excludes only the child being edited
(exclude_child_id), so case-only edits and stay-duplicated renames
warn like any other collision; renaming to your own name stays
warning-free when no other child holds the name. Tests: new
case-only-collision test, stay-duplicated rename assertion, and a
split self-rename test; suite at 848 passed.

overseer added 1 commit 2026-09-15 14:13:50 -04:00

Review fix: edit_child/set_child_active update+readback atomic 5578507ce8

Review round 2 (Codex gpt-5.6-terra) found both functions returned a
readback that a concurrent writer could overwrite between the UPDATE
and the SELECT, so the returned record could be the other caller's
values. Both now run existence-check, duplicate scan, UPDATE and
readback inside the DAOs' shared connection-scoped lock. Two gated
concurrency tests prove the contract deterministically: task A paused
at its UPDATE while task B queues, each returns its own values, final
stored state is the last writer's. Suite: 850 passed.

overseer added 1 commit 2026-09-15 14:15:00 -04:00

Review fix: race tests read via the DAO, not raw SQL probes 09363a57cf

The children/admin_users leak guard forbids children SQL outside the
DAO and its sanctioned test files; the two gated concurrency tests
probed the children table raw. They now assert via ChildrenDao.get.
Suite: 850 passed.

overseer added 1 commit 2026-09-15 14:18:36 -04:00

Review fix: serialize create_child's duplicate scan with the insert d5d972b872

Review round 3 (Codex gpt-5.6-terra) found two concurrent creates of
the same normalised name could both pass the duplicate scan before
either INSERT committed, inserting a duplicate with no warning. The
scan and INSERT now run inside the shared connection-scoped lock, so
the second create always sees the first's committed row and warns.
A gated concurrent-create test proves exactly one warning fires; a
sequential-duplicate warning assertion rode along. Suite: 851 passed.

dev_agent merged commit 6108755f0c into feature/4f802471-1de1-4c35-844f-eebf3bea0de5-children-registry-admin-allowlist

2026-09-15 14:20:25 -04:00

review_agent commented

2026-09-15 14:20:46 -04:00

Independent review — Codex gpt-5.6-terra

Round 1 — CHANGES_REQUIRED (1 MEDIUM): case-only edit could suppress the duplicate-name warning even when a different child already carried the normalised name. Fixed in 712527d: the scan always runs and excludes only the edited child; tests added (case-only collision, stay-duplicated rename, split self-rename).

Round 2 — CHANGES_REQUIRED (2 MEDIUM): edit_child and set_child_active returned a readback that a concurrent writer could overwrite between UPDATE and SELECT. Fixed in 5578507: check → update → readback run inside the DAOs' shared connection-scoped lock; gated concurrency tests added for both.

Round 3 — CHANGES_REQUIRED (1 MEDIUM): concurrent create_child calls could both complete the duplicate scan before either INSERT, inserting a duplicate with no warning. Fixed in d5d972b: scan + INSERT serialized on the same lock; gated concurrent-create test added (exactly one warning).

Round 4 — APPROVED (zero findings). Full suite verified by the reviewer: 851 passed. Merged via merge commit 6108755.

## Independent review — Codex gpt-5.6-terra **Round 1 — CHANGES_REQUIRED (1 MEDIUM):** case-only edit could suppress the duplicate-name warning even when a different child already carried the normalised name. *Fixed in 712527d:* the scan always runs and excludes only the edited child; tests added (case-only collision, stay-duplicated rename, split self-rename). **Round 2 — CHANGES_REQUIRED (2 MEDIUM):** edit_child and set_child_active returned a readback that a concurrent writer could overwrite between UPDATE and SELECT. *Fixed in 5578507:* check → update → readback run inside the DAOs' shared connection-scoped lock; gated concurrency tests added for both. **Round 3 — CHANGES_REQUIRED (1 MEDIUM):** concurrent create_child calls could both complete the duplicate scan before either INSERT, inserting a duplicate with no warning. *Fixed in d5d972b:* scan + INSERT serialized on the same lock; gated concurrent-create test added (exactly one warning). **Round 4 — APPROVED (zero findings).** Full suite verified by the reviewer: 851 passed. Merged via merge commit 6108755.

dev_agent referenced this pull request

2026-09-15 16:32:30 -04:00

Deactivate and reactivate a child without data loss (Feature 03, task 2) #52

dev_agent referenced this pull request

2026-09-15 17:38:23 -04:00

Feature 03: Children Registry and Admin Allowlist -> dev #56

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Maniacs_Mansion/NestQuest#51