CampusCore Coding Principles

These are the CampusCore-specific rules for planning, writing and reviewing code. General failure modes live in robust-code.md, and design guidance lives in the software_design_philosophy skill. Where that skill and this doc disagree, this doc wins.

These principles are the target, not a description of every file

Code that breaks these principles is not a template to copy, and the principles win over the style of the file around them. When a file you edit breaks one, bring your own change in line and tell the user what is wrong and what the right shape is. Refactor the old code only when the user agrees or the fix is proportional to the task.

Design Principles

Each principle names a kind of bug we shipped, and each one overrides convenience. Check every new feature against them before merge.

1. One service, one capability, one typed contract. A service takes one named Pydantic input, returns one named Pydantic output, and does one thing. A writer that also extracts has two jobs and no clear contract, so split it.

2. The orchestrator is the only place that knows step order. Services never know what runs before or after them. The orchestrator reads top to bottom and matches the state machine's STEP_ORDER one to one.

3. Each pipeline step is its own state-machine transition. Never hide several stages inside one opaque run_step call. Each step needs its own retry, its own cost row in DocumentIngestionRunEvent, and its own step_started and step_completed events.

4. No silent fallbacks for primary behavior. If feature X is the point of the work, X runs or raises a categorized error: SchemaInvalid goes to the Failed Items Inbox, Transient to the retry budget, and Fatal to an operator. Catch only the exception types you can handle, because a catch-all that returns a default keeps a feature broken behind green tests.

5. A green test must mean the feature works, not that a helper ran. The evidence that a change is done is a test of its public contract, such as "after ingest(folder_upload), typed Documents exist for the uploaded files." A helper test can pin tricky logic but never replaces the contract test, and a feature whose public API has no contract to assert is not done.

6. No plan or phase references in code or data. Never put Phase 2, v1, WIP, Stage 1 or similar in comments, names, fields, enum values, string literals or stamped data, because git history owns that. Outside Python, a running system's own stage names, such as a Docker build stage, are allowed, and scripts/check_comment_hygiene.py enforces the Python ban.

7. Feature gates are decisions, not hiding spots. Flipping a flag must change behavior end to end, and a test must assert that. Delete a flag in the same PR that makes it inert.

8. One service, one front door. A service module exposes one class with a constructor and one or two public methods, or one module-level function, and everything else is _underscored and absent from __all__. Two co-equal public functions in one module mean two services are fused, so split them.

9. Shared infrastructure lives in exactly one place. LLM clients, AWS clients, content-type resolution, token counting and tempfile lifecycle live in apps/main_app/clients/ and apps/main_app/utils/, never as private helpers inside a consumer. A def _llm(): or def _s3_client(): inside a service is a copied recipe that will drift.

10. A service is defined by its output, not by what callers do next. Persistence, telemetry, notifications and queue dispatch belong to the caller that uses the value, not to the service that produces it. Orchestrators compose services, and services never compose each other.

11. Leading underscore means genuinely internal. A _name is private to its class or its .py file, and production code never imports another module's _name, because lint cannot see that access. When a second file needs one, make it public, give it a real seam (a constructor parameter, a typed Settings field), or move the caller into the file. Builders pass state through __init__ so ruff's SLF001 stays silent, and an unavoidable exception, such as Django's Model._meta, is exempted in config or with # noqa: SLF001 and its reason. Tests may read privates, but setting or patching one needs a real seam or a comment marked Justified exemption under #35.

12. Exceptions live where their contract lives. An exception that a second subsystem catches lives in apps/main_app/services/error_handling/, and the typed-API exceptions stay in campus_core/api/exceptions.py. An exception raised and handled inside one subsystem stays at its raise site.

13. A name is a promise of exactly what the thing does. When a name and its behavior disagree, such as a get_* that creates, fix the name or narrow the behavior in the same change, because prose explaining the mismatch is not a fix. The rule covers admin action labels, URL names and CLI commands too, and a rename that users or the wire can see is a behavior change the plan and PR call out.

14. Fix the cause, not the place it showed up. Put the fix in the shared code so every other caller gets it too, not in a special case at the one spot that was reported. If the same bug could appear somewhere else tomorrow, the fix is in the wrong place. A fix stays at one caller only when the plan says why the other callers cannot hit the bug.

Type-Driven Development

Pydantic models are executable documentation and the main guardrails of the system.

1. Types First, Logic Second. Define the Pydantic input and output models, then the signature, then the logic.

2. Eliminate Primitive Obsession. Use semantic types for domain concepts (EmailStr, Field(gt=0)), not raw str, int or bool. A field with a closed value set, such as a status, kind, scope or outcome, is its model's TextChoices class on a wire model, never str (see "Enumerated fields" in spa-and-api-framework.md).

3. Make Illegal States Unrepresentable. Use Literal fields and discriminated unions instead of optional fields whose combinations can be invalid.

4. Parse, Don't Validate. Validate untrusted data once at the boundary (HTTP, database rows, LLM output, queue messages) into a narrower model, and pass that model downstream instead of the raw dict. Downstream code trusts what the type guarantees and still checks facts the type cannot hold, such as a user_id that must resolve to a live row.

5. Explicit Method Inputs. Every argument has a type hint, and three or more arguments become a Pydantic input model that holds the validation.

6. Explicit Return Types. Never return a raw dict: write -> UserProfile | None, not -> dict.

7. Review the representation, new and touched. Before the code, ask three questions of every model, Pydantic shape, queue message or TypeScript state type the change creates, edits, writes or branches on, about the whole structure as it is today:

1. Impossible combinations - Which combinations of these fields are impossible, and what in the type or the database stops them? Two booleans, a nullable pair or a status with companion columns usually hide a missing enum or variant.
2. Second home - Where else is this fact stored, and which single named writer keeps the other copy in step?
3. Silent rename - If one of these values is renamed, does the compiler or the test suite see it? A str status, a metadata key read in six files or a dict[str, Any] the SPA parses by hand breaks silently.

Reading a field only to pass it through or display it does not count as touching it. A "no" that predates the change is still a finding: fix it when the fix is proportional and write it into plan.md, or defer it to a tracking issue or a named candidate. Green tests answer none of the three, because a shape that allows an invalid state passes every test that never builds that state. Reference shapes: WebPage.host in apps/main_app/models/scraping.py is a database-computed column with no second writer, and the AgentEvent union in apps/main_app/services/agent/events.py gives each event kind only its own fields.

Comments & Docstrings

These rules cover every comment in production code, in every language, templates, CSS and config files included. scripts/check_comment_hygiene.py enforces the mechanical subset on changed files, including an em dash in a comment and a {# #} comment that spans lines, and the code-reviewer agent checks the rest.

1. A comment states a contract, an invariant, or a why the code cannot express - otherwise it does not exist. Delete a comment that restates the code and a docstring that rewords the def line, and write the comment a subtle decision needs.

2. No Args:/Returns:/Attributes: sections that restate typed signatures. A parameter earns prose only for a constraint its type cannot express, such as units, ranges, ownership or coupling between parameters.

3. Present tense only. No history voice ("Previously", "was renamed from"), no PR or commit references and no provenance citations, because git owns the past.

4. One canonical home per invariant. State an invariant once, at its strongest enforcement point, and give every other site at most a one-line pointer.

5. Narration comments are an extraction signal. Turn # --- step 2: rerank --- banners into named methods, or name a refactor candidate.

6. A behavior change includes its docstring in the diff. A stale docstring sends the next reader confidently in the wrong direction.

7. Runtime-consumed prose is exempt - edit it as contract content, never delete it as documentation. This covers Field(description=...) strings, connector @action docstrings, docstrings of Pydantic models whose schema reaches an LLM (the metadata-extraction schemas and agent tool args_schema models), Terraform description fields and the text a shell script's usage() prints. API view and API model docstrings feed the generated OpenAPI types, so re-run scripts/sync_openapi_types.sh after editing one.

Where shared code lives: campus_core/shared_utils/

A module belongs here when it has at least two real call sites, depends only on django.conf, the standard library and third-party packages, and does no database access at import time. Send workflow notifications through notify_workflow_event(channel_key=..., header=..., body=..., fields=...) in slack_utils.py, with channel keys from settings.NOTIFICATION_CHANNELS. It never raises, so a Slack outage cannot break a workflow.

Testing: prove behavior, not coverage

A change is done when a test asserts the observable contract it affects (Design Principle #5), not when every function has a test file. Do not mock away the collaborator whose contract you just changed, because the test stays green while production breaks. Tests live in campuscore_app/apps/<app_name>/tests/: pytest for agent and service tests, Django TestCase for view and model tests, and @pytest.mark.asyncio for async tests.