Skip to content

Bus shifts: model, constraints and a reproducible environment - #78

Draft
suush wants to merge 9 commits into
Optiways:masterfrom
suush:feature/srs/technical-review
Draft

Bus shifts: model, constraints and a reproducible environment#78
suush wants to merge 9 commits into
Optiways:masterfrom
suush:feature/srs/technical-review

Conversation

@suush

@suush suush commented Aug 29, 2026

Copy link
Copy Markdown

Honest status first

The exercise is not finished. The models, their constraints and their tests
are in place; the Django admin interface for BusShift is not, which means
objective 2 of the subject is not met. The time budget went further than the
four hours suggested, and the work stops here rather than being padded.

What is missing is listed precisely below, with what each piece would take. The
same applies to everything deliberately left out: docs/coverage.md states what
the system handles and what it does not, with the reason — an unhandled
scenario absent from that file is indistinguishable from one nobody thought
about.

Tests 24 pass, 1 red on purpose (make test)
Lint, formatting, deployment checks, docs build green (make lint, make format-check, make check, make docs-build)
Type checking 2 errors, in inherited code — see below
Admin for BusShift not implemented

The red test is not deleted. It asserts the "at least two stops" rule, which has
no implementation yet, and it carries a TODO pointing at where that rule will
live. A suite made green by removing what fails is a suite that measures
nothing.


Use of an AI assistant

An AI assistant was used for this exercise, and this section is here so that its
role can be judged rather than guessed.

The assistant was install from a personal owned project called Claude graft it's purpose
is to initialize an assistant on a project with a half project doctrine embeded, the other half is determined
at installation time of this graft plug in claude.
For example it already embed the DDD layering, the documentation rules, the quality rules .. etc
You can access it here: https://github.com/Forge-Stack-Workshop/claude-graft

It was bounded by configuration, not by good intentions. The assistant had
read-only access to the Django code: Edit(/padam_django/**) is deny in
.claude/settings.json, so it could not write a model, an admin class or a
migration. Proposals were delivered as code blocks in the conversation and typed
by hand. Two narrow exceptions were opened explicitly, each recorded in the
configuration with an expiry, and both closed before the end:

  1. a docstring pass over the inherited code (Bus, Driver, Place,
    User, the seeding commands) — comments only, no logic;
  2. an import-ordering and type-annotation pass across the codebase.

BusShift, BusStop, their constraints and their migration were written by
hand. git blame shows it.

The configuration is committed, deliberately.claude/ is not gitignored
in this repository. It contains the engineering doctrine the work follows
(.claude/rules/), the scope rule that bounded the assistant
(.claude/rules/interview-scope.md), the permission set, and
.claude/quality-gate.json, which records for every command whether its output
was actually seen or not.

The full session transcript is committed: docs/sessions/2026-08-29-d0f0905d.md.
It shows the reasoning, the disagreements, and the places where the assistant's
proposals were rejected or corrected.

The assistant's role was sparring: reviewing, flagging when a project rule was
about to be broken, and doing the parts that are not code — documentation,
environment, tests. Every design decision was taken by the author.


What was done, and why

1. The stack was moved off end-of-life versions

The repository shipped Django 4.2.16 and recommended Python 3.9.

Was Support ended Now
Django 4.2.16 7 April 2026 5.2 LTS — supported to April 2028
Python 3.9 (README) / 3.7 (Pipfile) 31 Oct 2025 / Jun 2023 3.13
Database SQLite PostgreSQL 17

This is a deliberate departure from the stated stack. Choosing runtime versions
is part of shipping something safely, and delivering an image built on an
unsupported runtime while claiming production intent would be incoherent.

The upgrade cost one line of code: USE_L10N was removed in Django 5.0.
unique_together was unaffected — it is index_together that went. Nothing
else in the deprecation list is exercised by this codebase.

It also earns two things the feature uses directly: GeneratedField (Django 5.0)
to have the database compute the shift's time range, and
UniqueConstraint.violation_error_message now always honoured, which turns a
constraint refusal into a readable message instead of an IntegrityError.

2. The environment was rebuilt to be reproducible

The project ran on the host in a virtualenv, and the README documented a
make migrate target that did not exist in the Makefile.

  • Multi-stage Dockerfile — production image, non-root, static files
    collected at build time, its own HEALTHCHECK, gunicorn.
  • Dockerfile.dev derives from it through a compose build context rather
    than repeating its FROM, so dev and production resolve one dependency set.
  • pyproject.toml + uv.lock as the single manifest; requirements.txt
    and Pipfile removed.
  • PostgreSQL 17 in compose, for dev/prod parity — and because the overlap
    constraint needs a database that can express it.
  • Makefile as the only human interface; no recipe runs a tool on the host.
    make up works from a fresh clone with no manual step: it creates .env from
    the committed .env.example.
  • Settings read from the environment, with no default for a secret. The
    application refuses to boot without DJANGO_SECRET_KEY or DATABASE_URL.
  • MkDocs site, built and served in the container.

Every command was executed and its output read. The two URLs the README hands
over were requested, along with every asset they reference.

3. BusShift and BusStop

The overlap constraint is enforced by the database, not by the application.
This is the central decision, and it drove the rest.

A clean() check does not survive concurrency: two transactions each validate
against a state that excludes the other, and both commit. Row locking does not
help either — the conflicting row does not exist yet. Only a database constraint
holds.

So BusShift carries stored starts_at / ends_at columns, recomputed from
its stops, and a period GeneratedField producing a TSTZRANGE. Two
ExclusionConstraints — one on the bus, one on the driver — refuse any overlap.

Three points worth raising in review:

  • Two constraints, not one on the pair. A shift may share its bus with one
    shift and its driver with another. A constraint written on (bus, driver)
    would let a bus be in two places at once.
  • The interval is half-open, [starts_at, ends_at). A shift ending at 12:00
    and one starting at 12:00 do not overlap, so a driver's day chains without an
    arbitrary one-minute gap.
  • The exclusion constraints are partial. A shift with no stops has NULL
    bounds, and TSTZRANGE(NULL, NULL) is the unbounded range, which overlaps
    everything — one stopless shift would forbid every shift for its bus. The
    condition= keeps those rows out of the index.

The cost is documented: stored bounds can drift from the stops. The
mitigation is a single explicit refresh_bounds()not a signal, because
bulk_create, bulk_update, queryset.update() and queryset.delete() all
bypass signals and are precisely the paths that would leave the columns lying.

An alternative was considered and rejected: making the bounds operator input
with stops required to fall inside. It removes the drift entirely, but inverts
the subject, which states that departure is determined by the first stop.

4. Documentation

  • docs/coverage.md — what is handled, what is not, and why not.
  • docs/flows/bus-shift.md — the call chain, written from the code, naming the
    missing admin link rather than describing an interface that does not exist.
  • docs/flows/existing-system.md — the inherited system, which surfaced three
    fragilities: a non-unique username factory against a unique column, seeding
    commands that create disjoint sets of users, and an N+1 in UserAdmin.
  • Google-style docstrings throughout.

Next steps — to finish the feature

  1. BusShiftAdmin with a BusStop inline. Objective 2. Without it a shift
    can only be created from the shell.
  2. save_related() calling refresh_bounds(), so the bounds are recomputed
    after the inline formset has written the stops — the only moment they are
    knowable.
  3. Formset clean() requiring at least two stops. This cannot be a table
    constraint: it counts rows in another table, and the zero-stop state is
    legitimate mid-transaction. The failing test is waiting for it.
  4. get_queryset() with select_related('bus', 'driver__user') and
    annotate(Count('stops'))
    , so the changelist costs three queries instead
    of three per row.
  5. Catch IntegrityError in the admin form and render the
    violation_error_message the constraints already carry, instead of a 500.
  6. autocomplete_fields and search_fields on bus, driver and place —
    a plain select over every place does not scale.
  7. A map widget for Place, so an operator picks a location rather than
    typing coordinates.
  8. Fix the two type errors in padam_django/apps/users/admin.py:
    is_driver.boolean and .short_description are function attributes, which
    are not typable. @admin.display(boolean=True, description="Is driver")
    replaces both.

Next steps — to go further

Why it is not here
GitHub Actions workflow — three lines: checkout, make ci make ci already exists as the single definition of green; only the thin caller on the forge is missing. Left out for time.
pre-commit hooks They would duplicate the gate definition for a budget that did not allow verifying both. Their hooks should be thin callers of the same make targets.
git-cliff + generated CHANGELOG.md Commits are already Conventional Commits, so the input exists. A cliff.toml with no release pipeline behind it serves nothing today.
Concurrency tests under real parallelism The constraints hold by construction, but no test opens two connections to prove it. TransactionTestCase plus threads is the shape.
Property-based testing of the bounds invariant The oracle already exists as a test helper; the missing piece is a generator of operation sequences (Hypothesis) to explore the paths nobody thought of.
Hexagonal / DDD layering Rewriting four Django apps into domain / application / infrastructure costs more than the exercise is worth, and would obscure the Django idiom being evaluated. BusShift would become a pure entity holding the overlap rule, behind a repository port.
Observability No structured logging, no correlation id, no spans.
Explicit authorization Django admin's is_staff is the only gate; no per-action role check, no denial test.
factory-boy and faker ship in the production image Accepted here because the create_* commands live in the application package. In a real deployment this is an architectural defect — seeding tooling must not ship with the application.
src/ layout and a PEP 517 build backend Required by the project's own Python conventions; moving manage.py and padam_django/ costs more than it returns for an application nobody installs as a package.

Reviewing it

make up        # PostgreSQL + the application, waits for health
make migrate
make seed
make superuser

Then http://localhost:8000/admin/. make docs serves the documentation on
:8001. make help lists every target. Nothing runs on the host.

suush added 9 commits August 29, 2026 19:26
The project ran on the host in a virtualenv, and the README documented a
`make migrate` target that never existed. A fresh clone now needs a container
runtime and nothing else.

Dockerfile.dev derives from the production image through a compose build
context rather than repeating its FROM, so dev and prod resolve one dependency
set. Static files are collected at build time and the image carries its own
healthcheck, so it serves without a second service.

make ci is the single definition of green; every other target is runnable
alone.
Django 4.2 left extended support on 7 April 2026 and Python 3.9 reached
end-of-life on 31 October 2025. Securing what is shipped includes the runtime
versions, so both move to the current LTS and a supported interpreter.

The upgrade costs one line in the codebase: USE_L10N was removed in Django 5.0.
unique_together is unaffected.

SQLite is replaced by PostgreSQL 17 in the container, for dev/prod parity and
because an overlap constraint can then be enforced by the database rather than
by the application.

pyproject.toml becomes the only manifest, with uv.lock as the source of truth.
requirements.txt and Pipfile are removed.

BREAKING CHANGE: settings are read from the environment and no longer carry
defaults. DJANGO_SECRET_KEY and DATABASE_URL are required and the application
refuses to boot without them. Every variable is listed in .env.example.
The README described a start-up procedure that no longer exists. It now lists
the URLs reachable after `make up` and what each one serves.

docs/coverage.md states what the system handles and, above all, what it does
not, each with its reason: the overlap constraint, the absent tests, the
deferred hexagonal layering. A scenario missing from that file cannot be told
apart from one nobody thought about.

docs/flows/existing-system.md is written from the code and records three
fragilities found while reading it, including a non-unique username factory
against a unique column.

Google-style docstrings document what the signatures do not say.
A BusShift carries a bus, a driver, and time bounds derived from its stops. A
BusStop is a place and a time of call, belonging to exactly one shift.

The non-overlap rule is enforced by two ExclusionConstraints — one per resource,
not one on the (bus, driver) pair, so a bus shared with one shift and a driver
shared with another are both caught. A clean() check would not hold: two
concurrent transactions each validate against a state that excludes the other,
and row locking cannot lock a row that does not exist yet.

The bounds are stored rather than derived on read, because an ExclusionConstraint
needs indexable columns. A GeneratedField turns them into a half-open TSTZRANGE,
so a shift ending at 12:00 and one starting at 12:00 chain without an artificial
gap. Both constraints are partial: a stopless shift has NULL bounds, and
TSTZRANGE(NULL, NULL) is the unbounded range, which would otherwise forbid every
shift for its bus.

The cost of storing them is a single explicit refresh_bounds(), deliberately not
a signal — bulk_create, bulk_update, queryset.update() and queryset.delete() all
bypass signals and are exactly the paths that would leave the columns lying.

24 tests pass. The one that fails asserts the two-stop rule, which has no
implementation yet: it counts rows in another table and belongs in the admin
inline formset. It is left red rather than deleted.

The admin interface is NOT implemented — see docs/coverage.md.
Standard library, third party, project, then a TYPE_CHECKING block last. The
order is enforced by ruff rather than agreed by convention: the TC ruleset moves
type-only imports into the block, and `from __future__ import annotations` is a
required import everywhere, so annotations are never evaluated at run time.

Public signatures are annotated. mypy runs non-strict: strict reports 52 errors
across the inherited code, and a gate that is red on every run teaches everyone
to ignore the gates. Tightening it is listed in docs/coverage.md.

Template tooling under .claude/ and the session transcripts are excluded from
ruff: neither is this project source, and the formatter rewrites Python blocks
inside markdown.
docs/coverage.md moves seven scenarios to the handled side and rewrites the
gaps to name what blocks objective 2: no BusShiftAdmin, no inline formset, no
recompute wired to save_related, no N+1 protection on the changelist.

The decision record explains why the bounds are stored rather than derived, what
that costs, and which alternative was rejected and why.

quality-gate.json records, for every command, whether its output was actually
seen. Nothing is called green on inference.
The body describes the branch, so committing it would date the moment it merges.
The entry point sits at the repository root, outside padam_django/, so it was
missed by a path-scoped add in the previous commit.
@suush
suush marked this pull request as draft August 29, 2026 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant