From af81827b14e75b6fe5942b8fef2437599c2abd4f Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Mon, 13 Jul 2026 15:38:42 +0000 Subject: [PATCH] docs: ADR-0013 tagging system + CONTEXT (Bulk tag assignment, Run filter tags) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the grill-with-docs outcome: flat app-owned Tags, four assignment paths (incl. in-app bulk upload, no FastAPI), tags as a Run-filter key extending ADR-0008, hard-delete + cascade, reporting-by-tag deferred to the reporting redesign. Design only — no implementation yet. Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 6 +- ...3-tags-are-app-owned-property-groupings.md | 101 ++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 docs/adr/0013-tags-are-app-owned-property-groupings.md diff --git a/CONTEXT.md b/CONTEXT.md index 628c0c25..aa87bc9a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -113,7 +113,7 @@ One user-initiated act of triggering modelling: a set of Scenarios crossed with _Avoid_: job, batch, trigger request, modelling task (the pipeline's execution records) **Run filter**: -The property-selecting constraints of a Modelling run: postcode, property type, and built form (each multi-select; tags later). An absent filter means *unconstrained* — "all postcodes" is the absence of a postcode filter, never an enumeration. Type and built form are the canonical enum vocabularies plus **Unknown**, which selects properties with no resolvable value at all. Resolution follows the Landlord-override rule: an override fact beats an EPC-derived value; the modelling backend is the single authority for resolving filters to properties (the app never re-implements the rule — matched counts come from the backend). +The property-selecting constraints of a Modelling run: postcode, property type, built form, and **Tags** (each multi-select). An absent filter means *unconstrained* — "all postcodes" is the absence of a postcode filter, never an enumeration. Type and built form are the canonical enum vocabularies plus **Unknown**, which selects properties with no resolvable value at all; Tags are the Portfolio's own Tags (multi-select is **any-of**). Resolution follows the Landlord-override rule for type/built-form (an override fact beats an EPC-derived value) and a **direct membership lookup** for Tags — but in all cases the modelling backend is the single authority for resolving filters to properties (the app never re-implements the rule — matched counts come from the backend, which reads the same Tag membership the app writes). See [ADR-0013](./docs/adr/0013-tags-are-app-owned-property-groupings.md). _Avoid_: property query, segment, search (the postcode-search journey is unrelated) ### Live projects @@ -132,6 +132,10 @@ _Avoid_: bulk export, document export, zip download A user-created, coloured label owned by a Portfolio, used to form an arbitrary **group** of its Properties for bulk actions (e.g. running Scenarios over the group, or excluding it from a Reporting view). Flat (no hierarchy) and many-to-many: a Property carries zero or more Tags, and a Tag labels zero or more Properties. Distinct from a **Project** (a HubSpot-sourced grouping, at most one per Property) and from a **Scenario** (a modelling question): a Tag is an app-owned, hand-curated selection with no external source and no modelling semantics of its own. _Avoid_: label, category, group (the group is the *effect* of a Tag, not a separate entity), segment, tag group +**Bulk tag assignment**: +Assigning one Tag to many Properties at once from a small uploaded file (CSV/Excel) of property identifiers — either **landlord property id** or **UPRN**, whichever column the file carries. Parsed and matched entirely in the app (no FastAPI pipeline); the target Tag is chosen in the upload dialog, not named in the file. An identifier that matches no Property is reported back (never silently dropped); an already-tagged match is a no-op. The parallel filter-driven path assigns a Tag to **every Property matching the current property-table filter**. +_Avoid_: tag import, bulk tagging (verb is fine; the noun is Bulk tag assignment) + ## Lifecycle A **BulkUpload** moves through these statuses: diff --git a/docs/adr/0013-tags-are-app-owned-property-groupings.md b/docs/adr/0013-tags-are-app-owned-property-groupings.md new file mode 100644 index 00000000..e139908a --- /dev/null +++ b/docs/adr/0013-tags-are-app-owned-property-groupings.md @@ -0,0 +1,101 @@ +# 13. Tags are app-owned Property groupings, filtered in-app and passed to modelling as a filter key + +Date: 2026-07-13 + +## Status + +Accepted + +Numbering note: 0010 (reporting redesign) and 0012 (portfolio-list descriptor +resolution) are accepted on their own in-flight branches; 0013 avoids collision +with both. + +## Context + +Users need to form **arbitrary, hand-curated groups** of a Portfolio's +Properties to drive bulk actions — running Scenarios over a subset, and (later) +including/excluding a group from Reporting. Nothing existing covers this: + +- **Projects** (ADR: HubSpot-sourced) are at most one per Property and owned by + an external system — not user-curated, not many-per-property. +- **Run filters** (ADR-0008) select by postcode / property type / built form — + data-derived facts, not a free-form selection. + +ADR-0008 already anticipated this: it kept the distributor's filter payload a set +of tiny keys resolved against the shared Postgres, **explicitly rejected sending +resolved property-id lists** (~50k ids overflow the async entry point and move +resolution into the app), and reserved that "tags become one more optional +filter key." + +## Decision + +- **A Tag is a flat, app-owned, Portfolio-scoped label**, many-to-many with + Property (`portfolio_tag`, `property_tag`). No hierarchy, no external source, + no modelling semantics of its own — it exists only to name a group. See + CONTEXT.md (Tag). +- **Assignment has four paths, all app-owned:** a per-Property inline editor; a + filter-driven "tag everything matching the current property-table filter"; + a row-multiselect (whose "select all matching" delegates to the filter path); + and a **Bulk tag assignment** file upload. +- **The bulk upload is parsed and matched entirely in the app — no FastAPI + pipeline.** The BulkUpload pipeline exists to match *arbitrary addresses* to + UPRNs and to *classify free text*; tag assignment matches an *existing + identifier* (landlord property id **or** UPRN, whichever column the file + carries) to *existing* Properties in one Portfolio. There is no address + matching and no classification, and the files are small — so the pipeline's + cost (S3 round-trip, async task, combiner) buys nothing. An unmatched + identifier is reported, never silently dropped; an ambiguous landlord id (they + are **not** unique per Portfolio) tags every match and says so. +- **Tags reach modelling as a `tag_ids` filter key (extends ADR-0008), never as + a property-id list.** `RunFilters` gains `tag_ids`; it is **AND-combined** + with the other filter dimensions and **OR-within** (any-of), matching the + property-table filter. The app computes the in-app preview count by joining + `property_tag` (the reference implementation ADR-0008 requires); the + distributor resolves the same key by the same join against the shared DB it + already reads. This needs one backend change — the distributor learning + `tag_ids` — so the app side ships first and scenarios-on-tags goes live when + the backend lands. +- **The property-table Tag filter is app-resolved** (unlike modelling, the app + owns the browse surface): a dynamic per-Portfolio option list, any-of, plus a + synthetic **Untagged** option (zero memberships). +- **Delete is hard + cascade.** Removing a Tag drops its memberships behind a + confirm that shows the count. A Tag id baked into a past Run's stored filter + becomes dangling and renders as "(deleted tag)" in Run history — no archive + state to query around. +- **Schema migrations ship in their own PR**, separate from the feature code + (project convention). + +## Alternatives considered + +- **Two-level Tag Group → Tag hierarchy.** More expressive (mutually-exclusive + groups), but every stated use case is served by flat Tags with far less schema + and UI; grouping can be added later without reshaping the join. +- **Reuse Projects as the grouping.** No new tables, but Projects are + HubSpot-owned, one-per-Property, and not user-editable — the opposite of what a + Tag is. +- **App resolves `tag_ids` → property-id list for the run.** No backend change, + but re-runs exactly what ADR-0008 rejected: an unbounded payload and the + resolution rule living in the app, where preview and run can drift. +- **Route the bulk upload through the FastAPI BulkUpload pipeline.** Reuses the + existing async machinery, but pays for address-matching and classification the + task doesn't need, and couples a simple in-Portfolio identifier join to a + multi-stage S3 pipeline. +- **Soft-delete / archive Tags.** Preserves history references exactly, but + burdens every Tag query with an `archived` predicate for a gain a + "(deleted tag)" label already covers. + +## Consequences + +- **One backend ask** must land for scenarios-on-tags to work end-to-end: the + distributor accepts and resolves a `tag_ids` filter key against `property_tag`, + mirroring the app's preview. Until then the run UI can show and preview a Tag + selection but the run won't scope by it. +- **Reporting-by-tag is deferred to the reporting redesign** (PRD #370 / + ADR-0010): the substrate (`property_tag`) ships now; "include/exclude a tagged + group" becomes a requirement of that redesign rather than a retrofit onto the + scenario-aggregate reporting it replaces. +- A deleted Tag leaves a dangling id in any Run-history record that referenced + it; history reads must tolerate it (render "(deleted tag)"). +- `landlord_property_id` is partial (~43% of Properties) and non-unique per + Portfolio, so bulk uploads that use it cover less than half the Portfolio and + can tag duplicates — UPRN is offered as the reliable alternative key.