Skip to content

refactor(core): apply constructive domain modeling to workflow state #1205

Description

@phernandez

Goal

Adopt Constructive Domain Modeling incrementally in workflows where the current representation admits unsupported states or uses exceptions for expected domain outcomes.

The style means:

  • define the domain's positive space with small required-field values;
  • use frozen dataclasses for internal products and closed type unions for meaningful alternatives;
  • use Pydantic at API, MCP, CLI, configuration, and persistence boundaries;
  • parse/classify once, then handle variants exhaustively;
  • keep cancellation and unpredictable filesystem, network, queue, and database failures exceptional.

This is a tracker for focused, behavior-preserving slices—not one sweeping refactor.

Evidence and first candidates

1. Note-content reconciliation is close, but its contract is still broader than its callers

The planner already returns a closed union of frozen plan values, and the apply path already uses exhaustive matching with assert_never.

However, plan_note_content_reconciliation(current: NoteContentState | None, ...) returns the full union for both input cases. Callers therefore need runtime guards for two combinations the implementation cannot produce:

This is the best first pilot because the constructive model already exists and the remaining change is small and type-checkable.

2. Persisted note-content state is broader than the domain transitions

NoteContentState combines a write status with optional file version/checksum fields. That may admit combinations no workflow should construct.

This must begin with a writer and compatibility audit. The ORM/storage shape may need to remain broad while a parser constructs narrower internal variants.

3. Project lifecycle responses combine unrelated operations

ProjectStatusResponse represents create, update, default-switch, and delete responses through one model with several operation-dependent optional fields.

Project deletion already has a stronger internal accepted result, but expected rejections are carried by an exception with HTTP status and detail. That mixes a normal domain decision with its HTTP presentation.

Proposed sequence

Phase 0 — land the vocabulary

  • Land the Constructive Domain Modeling section in docs/ENGINEERING_STYLE.md.
  • Land the corresponding Write, Refactor, and Review rules in .agents/skills/pythonic-code/SKILL.md.

Phase 1 — make note-content planner obligations visible to the type checker

  • Introduce a plan alias for existing note content that excludes NoteContentBootstrap.
  • Add overloads or split functions so None returns NoteContentBootstrap and NoteContentState returns only existing-state plans.
  • Remove the two runtime “must/cannot bootstrap” guards once static checking proves the calls.
  • Preserve the current stale-anchor and concurrent-insert behavior; IntegrityError remains an external/concurrency failure.
  • Cover both input contracts and every returned variant in focused tests.

Suggested verification:

uv run pytest -q \
  tests/indexing/test_note_content_reconciliation.py \
  tests/indexing/test_note_content_reconciler.py \
  tests/indexing/test_note_content_batch_reconciliation.py
uv run ty check src tests test-int

Phase 2 — define valid note-content lifecycle states

  • Trace every writer of db_version, file_version, checksums, and file_write_status.
  • Document the valid state-transition table before changing types.
  • Decide whether a small union of lifecycle values or one validating parser removes concrete invalid combinations.
  • Keep the ORM and historical payload shape stable unless the audit demonstrates that a migration is both necessary and safe.
  • Add parser/transition tests for every supported persisted combination and reject malformed combinations at the boundary.

Phase 3 — separate project operation outcomes

  • Inventory all API, typed-client, MCP, and CLI producers/consumers of ProjectStatusResponse.
  • Define operation-specific internal values for create/update/default-change/delete outcomes.
  • Preserve the current JSON contract initially, or document and test an intentional versioned schema change.
  • Represent expected delete rejection (not found, only remaining project) as explicit variants and translate them to HTTP errors in the API adapter.
  • Keep queue, transaction, cancellation, and database failures as exceptions.
  • Add response-compatibility snapshots plus exhaustive internal outcome tests.

Phase 4 — find the next high-payoff slices

  • Audit for status strings coupled to optional fields, repeated invariant checks, “should never happen” branches, and exceptions whose callers make ordinary domain decisions from them.
  • Create a child issue only when it names the unsupported state, the proposed constructors/variants, the owning boundary, and the compatibility proof.
  • Record candidates that are already clear or dominated by external failures as “no change” rather than forcing the pattern.

Non-goals

  • Removing Pydantic; it remains the runtime boundary model.
  • Introducing a universal Result[T, E] abstraction.
  • Converting filesystem, network, queue, database, or cancellation failures into domain variants.
  • Adding wrapper-only ID types that do not remove a real invalid operation.
  • Reshaping ORM tables before tracing every writer and compatibility constraint.
  • Breaking public API/MCP/CLI payloads merely to make internal types look cleaner.
  • Mechanically rewriting clear procedural orchestration that does not admit invalid state.

Tracker acceptance criteria

  • The style documentation is merged and used by the Python authoring/review skill.
  • Each implementation phase lands as a focused PR or is split into a linked child issue with a narrower owner.
  • Closed unions have exhaustive consumers checked statically; catch-all matches do not hide new variants.
  • Boundary validation and serialized compatibility are covered by tests.
  • Each type change identifies the concrete invalid state or runtime guard it eliminates.
  • No phase broadens exception handling or changes external behavior without an explicit, tested contract decision.
  • Repository type checks and the targeted behavior tests pass for every landed slice.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcloudBasic Memory ClouddocumentationImprovements or additions to documentationduplicateThis issue or pull request already existsenhancementNew feature or requestgood first issueGood for newcomersquestionFurther information is requested

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions