isolating-product-facade-contracts

name	isolating-product-facade-contracts
description	Plan and execute product isolation migrations to a facade plus contract layer in PostHog, following the Visual review architecture. Use when a product still exposes internals (models/logic/views) across boundaries and needs migration toward contracts.py + facade/api.py + presentation separation, with a PR strategy that minimizes review latency and conflicts with parallel work.

Isolating a product with facade and contracts

Use this skill to migrate an existing product to the isolated architecture used by Visual review. Optimize for short calendar exposure, not small diffs: authoring is cheap and the verification chain catches mechanical breakage, while human review latency and a fast-moving master are the real bottlenecks. Default to one PR structured in reviewable commits; a separate facade-first PR exists only as the merge base for team-sliced sweeps — per the PR strategy below.

Prerequisite: the product must already live under products/<name>/. This skill does not cover moving code out of posthog/, ee/, or other shared directories — do that first.

Core docs to load first

Read these before changing code:

1. products/architecture.md
2. products/README.md
3. docs/internal/monorepo-layout.md
4. posthog/models/team/README.md (team extension model rule)
5. docs/published/handbook/engineering/type-system.md (serializer/OpenAPI type flow)
6. docs/published/handbook/engineering/ai/implementing-mcp-tools.md (schema quality and team isolation expectations)
7. .agents/security.md (SQL/HogQL security guidelines)

Use Visual review as the concrete reference implementation:

• products/visual_review/backend/facade/contracts.py
• products/visual_review/backend/facade/api.py
• products/visual_review/backend/presentation/views.py
• products/visual_review/backend/presentation/serializers.py
• products/visual_review/backend/logic/
• products/visual_review/backend/tests/test_api.py
• products/visual_review/backend/tests/test_presentation.py

Before changing code, get the baseline:

hogli product:maturity <name>       # scores models, facade, presentation, boundaries, codegen
hogli product:lint <name>           # structural lint + isolation chain (strict if facade/contracts.py exists)
hogli product:isolate:scan <name>   # the recon: import map, coupling gate, preflight (see below)

First, read the baseline to learn which migration phase you're in — you don't start knowing whether this is untouched or mid-flight, and that decides where you pick up. The signals are all in the output above; there is no status file to consult (a hand-kept one would drift — the maturity score plus the ignore_imports TODO set is the status, and lint-imports prunes the latter as modules finish):

• Fresh — facade/presentation score zero, no backend/facade/, no ignore_imports entries for the product. Start at step 1.
• Mid-sweep — facade/contracts.py + facade/api.py exist but core still imports internals (scan still lists model-access/etc., or product:lint warns has legacy interface leaks). The facade design is settled; continue migrating callers (step 4) — don't redesign it.
• Mid presentation-wave — the chain is mostly intact but pyproject.toml still has ignore_imports TODO entries for the product. The sweep is done; resume the presentation wave on exactly those deferred modules.
• Effectively done — chain complete, no ignore_imports entries left. Verify against the done criteria instead of changing anything.

The scan does the recon that would otherwise cost a dozen ad-hoc greps, and is the source of truth for the rest of the flow:

• References by kind — every cross-boundary reference to the product's internals, classified (model-access, query-runner, celery-task, temporal-wiring, test-fixture, string-reference) with the facade pattern each kind maps to. This is the sweep checklist; string references (@patch(...) mock paths, dotted names in config) are included, which import-oriented grepping misses.
• Core-coupling count — picks the PR strategy below: near zero means the whole migration fits the single-PR default; triple digits means the caller sweep needs slicing by owning team.
• Strict-lint preflight — product:lint switches from lenient to strict the moment facade/contracts.py exists; the scan runs the structural checks in forced-strict mode so those demands (root tsconfig.json, tasks.py at tasks/tasks.py, required root files in place) surface up front instead of mid-migration. Strict lint checks the required root files only. It does not restrict internal directory names: the enforced shape is the import chain — routes.py imports only presentation/, and presentation/ imports only facade/ — so a product may add other internal packages (services/, reviewer/, …) as long as they stay behind the facade.
• Thin/thick signal per view module — the future ignore_imports allowlist size.
• Blind spots — the scan reads backend.* imports only. A coupling count of zero is necessary, not sufficient: it can't see product-root packages core imports (dags/) or non-import channels (config dotted strings, hogql system tables, in-process API test dispatch). See "Clearing coupling the scan won't show" below before declaring a product done.

--json emits the machine-readable recipe — keep it with the PR; regenerating the migration against fresh master starts from a fresh scan.

When the tooling doesn't fit, fix the tooling

Don't quietly hand-work-around it. If isolate:scan/:move misses your product's shape — a layout it doesn't detect, a coupling channel it can't see — and other products share that shape, extend the tool and update this skill so the next migration inherits the fix. That's how backend/api/-subpackage support and the Dagster-assets channel got added. Reserve a manual work-around (called out in the PR) for a true one-off; a silent hand-hack just leaves the next product to hit the same wall.

"No in-process callers, so no facade" is the wrong test

The core-coupling count sizes the sweep — zero importers means a single-PR migration with no caller migration — but it does not decide whether to isolate. The facade is also the structural seal that makes the CI skip sound, and that has nothing to do with importer count:

• A product's HTTP API is exercised in-process by tests (the Django test client dispatches into the view stack in the same process, not over a real socket). Cross-cutting tests — permissions, schema, activity-log, "every viewset does X" — reach a product's endpoints by URL, coupling to its live behavior with zero imports. tach, lint-imports, and this scan all read the import graph, so none of them can see it. "No importers" is necessary, not sufficient.
• Because the channel can't be enumerated, it is closed by construction, not audit: keep presentation thin and reaching internals only through the facade, so every observable behavior lives in the facade (tested in-product, inside the boundary) or in the serializer shape (the OpenAPI schema, whose changes already force the full suite); and keep behavior tests in-product.

So a product whose only consumers are over HTTP (node services, the TS/MCP codegen) is not facade-optional — there the facade's whole job is sealing its own presentation (step 2's second demand source). The genuine exception is a product with essentially no Django-side logic (a thin shim over an external service): it has nothing to seal, but it is then simply not isolated — no backend:contract-check, still paying the full suite. That is an accept-the-cost choice, not "isolated without a facade".

This is the third non-import coupling channel, alongside hogql system tables and dotted-string config (step 6.4) — and the least visible, since the other two at least leave a string to grep for.

Clearing coupling the scan won't show

product:isolate:scan walks the import graph of backend.*. Four kinds of coupling escape it — none is a dead end, each has a defined move. After the backend sweep, git grep "products.<name>" (not just .backend) and read the scan's string-reference section to find them.

Test-infrastructure coupling. Core tests reach into the product's test helpers, which no facade re-export naturally covers:

• Monkeypatch targets — a core test base patches a product module attribute (e.g. posthog/test/base.py patches execute_hogql_query on each runner module). Re-export the module object through the facade (facade/queries.py re-exports web_overview, not just its class) and point the patch at the facade path.
• Shared fixtures / base classes — a core test subclasses the product's test base. Decide ownership: if the fixture is infrastructure for a core concern (a preaggregated-table test base, and the tables live in core), move it down into core and have both sides import it downward; if the core test is really exercising the product's behavior, relocate the test into the product. Either way the cross-boundary test import disappears.

Surfaces outside backend/. A product can expose non-Django surfaces at its root — Dagster assets under products/<name>/dags/, for instance — that core imports directly. The scan only walks backend.*, so it won't list them, and tach / product:lint fail late with "not part of the public interface" (a direct interface exposure also trips the legacy-leak check). Re-export them through a facade submodule (facade/dags.py re-exporting the asset modules) and reroute the core importer, exactly like the temporal and query-runner wiring.

No external data consumers. Covered above — the facade serves the product's own views. Provide facade read functions for its models; wire the cheap views; defer the expensive ones (nested-serializer or transactional viewsets) as named ignore_imports for the presentation wave. Providing the facade function while deferring its caller is a legitimate intermediate state, not a half-migration.

Product-owned HogQL system tables. When core mounts a product's federated system tables (schema/system.py, lazy_join_registry.py), answer two independent questions:

• Can the reference be a normal facade import? Yes — table defs and lazy-join functions are plain module-level objects; move them into the product (e.g. facade/hogql.py) and reroute core's import, like any other wiring.
• Do the objects enter the static pickled catalog? Core builds the catalog once and reloads it per request through a restricted unpickler (build_database_root_node in posthog/hogql/database/database.py). Any product-defined class in the catalog tree (a PostgresTable/LazyTable subclass) needs its module added to _CATALOG_PICKLE_MODULES — allowlisted individually, not by prefix. A missing entry fails the core catalog tests with a message naming the module. Warehouse-style per-team tables are built at request time and never enter the static catalog, which is why most products never hit this.

The web_analytics migration is the worked example of all three: its preagg test base moved down to core, its timezone integration test moved into the product, its Dagster assets gained a facade/dags.py, and its filter-preset reads landed in facade/api.py with the viewset deferred.

Model classes a consumer already holds

Some products still hand out model classes under the watched-models allowance (MODEL_CROSSINGS). That allowance only says the class may leave the product; it says nothing about what the consumer does with it. Two rules cover that:

• Default-deny. A crossing class may appear in consumer code only in a shape the check calls instance-free: an annotation, X.DoesNotExist, a nested class attribute (X.Status), X._meta, a manager chain ending in values/values_list/count/exists/aggregate, or a chain embedded in Exists(...)/Subquery(...). Anything else is disallowed.
• Move, don't permit. Code that queries, serializes or writes a model belongs in that model's product. The remedy for a disallowed use is a move; the facade function is what the move leaves behind, and the consumer keeps orchestration and ids.

hogli product:crossings <product> lists a product's crossing classes with every consumer use bucketed by kind, disallowed first. Disallowed uses are frozen in products/model_crossing_uses_baseline.txt and guarded by a repo-invariant test; counts may only go down, and hogli product:crossings --all --write-baseline records the decrease. See products/architecture.md § Wiring couplings.

Guardrails

• Keep facades thin; put business rules behind the facade, in logic/ by default. Other internal packages (services/, reviewer/, …) are fine as long as they stay behind the facade.
• Transaction boundaries belong in the facade (or logic), not in views.
• Never return ORM models across product boundaries.
• Keep contracts pure (no Django/DRF imports).
• Filter by team_id in querysets.
• Do not add product-specific fields to Team; use a Team Extension model.
• Add request/response schema annotations on viewset endpoints (@validated_request or @extend_schema).
• Regenerate OpenAPI/types (hogli build:openapi) when serializer/view changes affect API schema.
• Presentation may only reach internals via the facade — enforced by the presentation must use facade import-linter contract in pyproject.toml (tool.importlinter). Any new internal module (cache.py, helpers.py, …) is auto-covered; there is no blocklist to maintain. New cross-cutting imports must either go through the facade or be temporarily allowlisted there.

Required migration workflow

1. Build an import map for the target product.
   • Find cross-product imports into target internals (models, logic, presentation, non-facade modules).
   • Classify each usage by capability (read/list, detail/read, create/update/delete, async/task, webhook/event).
2. Define the contract surface from both demand sources.
   • External consumers (core, other products): start from currently consumed fields only.
   • The product's own presentation (step 5): the facade functions its views will call. For thick views this share may be deferred — but name the deferral; it decides whether the isolation you turn on is sound or optimistic (see step 5).
   • Create frozen dataclasses in backend/facade/contracts.py using pydantic.dataclasses.dataclass — same shape as the stdlib variant but with runtime type validation on construction.
3. Introduce a thin facade in backend/facade/api.py.
   • Map ORM instances to contracts with explicit mapper functions.
   • Keep method names capability-oriented and stable.
   • Wiring that core registers (celery beat tasks, temporal workflows/activities/metrics/ schedule constants, query-runner registry hooks, Dagster assets) crosses the boundary as objects, not data — give each its own facade submodule (facade/tasks.py, facade/temporal.py, facade/queries.py, facade/dags.py) re-exporting exactly what core touches. Registry-style consumers dispatch on class identity (isinstance), so re-export the class itself; monkeypatch consumers need the module object, so re-export that too. Keep facade/api.py free of heavy imports (HogQL, temporalio) so config-only consumers don't drag them onto the django.setup() path — and split light shared tables (facade/hogql.py) out of the heavy runner re-exports for the same reason.
4. Migrate callers in one pass by default.
   • The rewrite is mechanical (import swap + call mapping) and fully checked by the verification chain; batching it only stretches the window in which master drifts under the branch.
   • Core test fixtures that need product models: use apps.get_model("<app_label>", "Model") at runtime plus a TYPE_CHECKING import for annotations — tach ignores type-only imports.
   • Compatibility shims exist to bridge between serial PRs — the one-pass shape rarely needs them. If one is unavoidable, it dies in the final cleanup PR, not "later".

Legacy leaks during migration

This is the exception path for high-coupling products whose caller sweep is sliced across teams (see PR strategy below) — the tach boundary turns on before every core import has moved, while the backend:contract-check CI switch must wait until the block is gone (hogli product:lint rejects the script while leaks remain). With the single-PR default, all core callers land in the same PR and this block never needs to exist.

If posthog/ or ee/ still imports product internals (backend.models, backend.oauth, …) when the isolation chain turns on, add a second [[interfaces]] block under the "Legacy leaks" section of tach.toml allow-listing exactly the modules core still touches. This keeps the build green while the remaining team-sliced PRs land. Shrink and delete that block as imports move behind the facade — the final cleanup PR removes it entirely. hogli product:lint flags any product that still has legacy leak interfaces with a ⚠ has legacy interface leaks warning.

PR strategy

The scarce resources are reviewer attention and calendar time — not authoring effort. Every open migration branch rots as parallel work lands, and every extra PR adds a serialized review window plus a full-suite CI run (the product isn't isolated yet, so each migration PR pays the full suite). Use the fewest PRs the merge topology requires, and let commit structure — not PR boundaries — organize review.

Default: one PR, structured in reviewable commits.

1. Commit — facade. Contracts + facade (including wiring submodules) + behavioral-parity tests. Pure additions; reviewers read this commit as the design.
2. Commit — sweep. All caller migrations. Mechanical; reviewers sample, the verification chain carries it.
3. Commit — structure + chain. Presentation move and rewrites (hogli product:isolate:move), strict-lint fixes, tach interface. Add backend:contract-check + narrowed turbo.json here only if the product is fully isolated in this PR — no deferred view modules. If any module is deferred (ignore_imports entries remain), the skip can't turn on yet (product:lint blocks it), so it lands with the presentation-wave PR; this PR ships without the skip and keeps paying the full suite, which is sound.

Don't split the facade into its own PR for reviewability: new files cannot conflict regardless of which PR they sit in, so a standalone design PR buys no conflict protection — it only serializes two review windows and doubles the migration's CI cost. Worse, a facade with no callers can only be reviewed in the abstract; in one PR the caller diffs sit next to the design and justify (or indict) it. See user_interviews PR #59132 and the logs migration (#63180 + #63184, done as two before this was learned) for the shapes.

In the PR description, enumerate the behavior-bearing edits — write paths, transaction boundaries, serializer changes, anything renamed or with pinned compatibility (celery task names). That list is where review attention goes; everything else is sampled. If a write path is genuinely subtle, pull those few callers into their own small PR as the exception.

Treat the PR as regenerable, not rebasable: the import map plus the facade design fully determine the rewrite, so on conflict, regenerate the branch against fresh master instead of hand-resolving. Keep it open only for the review window.

A separate facade-first PR exists for exactly one reason: it is the shared merge base when the sweep must be sliced. Gate on the core-coupling count from the baseline:

• Low (zero to low double digits): single PR, as above; skip the legacy-leaks machinery entirely.
• High (triple digits): the sweep won't review as one unit. Land contracts + facade
  • parity tests first — that standalone review is meaningful here, because every slice is about to code against it — then slice the sweep by who owns the calling code (the consuming team/area, not the product's own owner), reviewed in parallel, never serially by capability. Add the legacy-leaks tach block (above) while slices land, but do NOT add backend:contract-check yet — hogli product:lint rejects that script while leaks remain. The final cleanup PR drops the block, ignore_imports TODOs, dead adapters, and shims, and only then enables contract-check + narrowed turbo.json inputs — the CI payoff for this path lands at the end.

Deferred view modules (step 5) add one follow-up on either path: the presentation wave — see the section below for its flow and outputs.

Trade-off to accept: one PR reverts coarser than several. That is fine because the mappers are behavior-preserving, the behavior-bearing edits are enumerated up front, and the whole PR is regenerable — weeks of serial PRs on a hot product cost more in conflicts and forfeited isolated-CI time than an occasional coarse revert.

Presentation wave

The second demand wave on the same facade: the functions the product's own views need, where the first wave served external consumers. Run it once per product, after the migration PR lands, working from the deferred modules listed in the PR (the scan's view-module table is the worklist).

Per deferred module:

1. Design the facade functions its views need. Query endpoints get run-functions that take posthog.schema query objects and return the runner's response types — those are already framework-free, so usually no new contracts. Model-backed endpoints get contracts plus CRUD/capability functions (the DataclassSerializer pattern from visual_review).
2. Draw the line: execution and transaction concerns (ExecutionMode, query tagging, select_for_update caps, cross-product side effects) move behind the facade; HTTP concerns (request validation, response codes, report_user_action analytics) stay in the view.
3. Thin the view to parse → facade → serialize, with parity tests for the new facade functions, same as wave one.
4. Delete the module's ignore_imports entries — lint-imports must pass without them. That deletion is the output that marks the module done.

This is a design PR, not a sweep: it carries the same review weight as the facade commit (the contract and function shapes are the long-lived API), so it gets its own PR rather than riding along with mechanical work. Refactoring a wave-one function while here (e.g. subsuming a runner builder under a run-function) is fine — its callers are few and the parity tests pin behavior. The product's done criteria below are only fully satisfiable once this wave has emptied the allowlist — and emptying it is what unlocks the skip: the PR that deletes the last ignore_imports entry is where backend:contract-check + narrowed turbo.json are finally added (product:lint blocks them until then). For a product that deferred modules, this is when it earns the CI skip.

Done criteria

Treat migration as complete only when:

• Cross-product imports use backend/facade only.
• Facade returns/accepts contracts, not ORM.
• Presentation layer no longer encodes business logic.
• Tests cover facade and presentation boundaries.
• The product is listed in the shared [[interfaces]] block in tach.toml exposing backend.facade.* and backend.presentation.views.* — no legacy leak block remains.
• tach check --dependencies --interfaces passes with no violations for this product.
• lint-imports passes with no ignore_imports entries left for this product in the pyproject.toml TODO section — entries are tracked architectural debt from deferred view modules; the presentation wave deletes them.
• hogli product:lint <name> shows no legacy leak warning and the isolation chain is intact.
• backend:contract-check is present in package.json with turbo.json inputs narrowed to backend/facade/** and backend/presentation/** (enables the CI skip). This is the last thing added — product:lint blocks it until the ignore_imports entries above are gone, so the skip only ever turns on for a fully isolated product.