RICS / backend /docs /RICS_CONSTANT_INVENTORY.md
StormShadow308's picture
Add demo documentation and Docker setup for v2 report generation system
aad7814
|
Raw
History Blame Contribute Delete
9.68 kB
# RICS Hardcoded-Constant Inventory
**Status: RESOLVED** β€” all three constants relocated/extracted; `test_no_rics_hardcodes`
now passes (suite stepped 3 β†’ 2 β†’ 1 β†’ 0, one constant at a time). The sections
below retain the original pre-refactor analysis and append the post-refactor
source-of-truth location and its guard test.
Scope: the three production modules that previously failed `test_no_rics_hardcodes`
because they hardcoded RICS-domain rating vocabulary.
> The refactor was a **pure relocation / schema-driven extraction**: no constant
> value and no runtime behaviour changed. Byte-for-byte identity (2.2) and exact
> set identity (2.1) are pinned by dedicated guard tests.
---
## 1. The guard test
`backend/tests/test_no_rics_hardcodes.py`
```python
_FORBIDDEN = re.compile(r"Condition Rating|CR[123]|_RICS_LETTER", re.IGNORECASE)
```
Parametrised over every `backend/**/*.py` **except**:
- anything under a `tests/` path
- anything under a `prompts/` path
- `rics_canonical_l3.py`
- `__init__.py`
**Architectural intent encoded by the exemptions:** RICS-specific literals are
permitted only in (a) the canonical schema seed (`rics_canonical_l3.py`),
(b) prompt templates (`backend/prompts/`), and (c) tests. Everywhere else,
domain vocabulary must be **derived from the discovered `TemplateSchema`**, not
inlined β€” so the engine stays firm-/template-agnostic.
---
## 2. The three violations
### 2.1 `backend/core/pii_scrubber.py` β€” `_STATUS_TERMS` (line ~58)
```python
_STATUS_TERMS = frozenset({
"condition rating", "defect", "satisfactory", "serviceable", "deflection",
"distortion", "cracking", "spalled", "damp", "moisture", "insulation",
})
```
- **Match that trips the test:** `"condition rating"`.
- **Role:** part of `PROPTECH_SAFE_WHITELIST` (Pass-2 guardrail). Whitelisted
spans are **never masked** as PII. Removing/renaming the term risks the
scrubber redacting the legitimate phrase "condition rating" out of reference
prose β€” re-introducing exactly the over-redaction class we just fixed.
- **Blast radius:** every REFERENCE-tier ingest (`RagStore.ingest_document` β†’
`scrub_reference_for_ingest`) and every `assert_no_pii` gate (DOCX export).
- **Load-bearing:** YES. Touching this changes what survives redaction globally.
- **RESOLVED β†’** `pii_scrubber._BASE_STATUS_TERMS` (the 10 non-rating terms) plus
the rating term derived as `DEFAULT_RATING_SYSTEM_NAME.lower()` imported from
`rics_canonical_l3.py`. `_STATUS_TERMS = _BASE_STATUS_TERMS | {rating}` β€” the
effective set is unchanged. No literal `"condition rating"` remains in the file.
Guard: `backend/tests/test_pii_whitelist_source.py` (exact set match +
canonical-derivation + the phrase still survives a scrub).
### 2.2 `backend/core/reference_mapper.py` β€” `_RICS_DOMAIN_RULES` (lines ~26–32)
```python
_RICS_DOMAIN_RULES = """
RICS LEVEL 3 DOMAIN RULES (mandatory):
- Condition ratings may only use: "1", "2", "3", "NI", "NA". Never emit an empty rating token.
- If a note or baseline fragment is ambiguous or corrupted, preserve it verbatim inside [AMBIGUOUS: <text>].
- Output pure continuous prose only. No markdown fences, bullet lists, or chat preamble.
- British English throughout.
"""
```
- **Matches that trip the test:** `"Condition ratings"` (line 28) and
`"condition rating field"` (line ~88, the per-call rating hint).
- **Role:** system-prompt fragment injected into the in-place mapping LLM call.
The allowed rating tokens (`1/2/3/NI/NA`) are RICS-L3-specific.
- **Blast radius:** the grounding/mapping LLM output contract. Changing the
allowed-values text changes what the model is told it may emit per section.
- **Load-bearing:** YES (prompt contract). NOTE: this is arguably mis-located β€”
prompt text belongs under `backend/prompts/` (which the test exempts), so a
large part of the fix is simply **relocation**, not redesign.
- **RESOLVED β†’** both fragments relocated **verbatim** to
`backend/prompts/mapping_prompt.py` as `RICS_DOMAIN_RULES` (the rules block) and
`RATING_HINT_TEMPLATE` (the line-88 hint). `reference_mapper._RICS_DOMAIN_RULES`
is now an alias to the relocated object; the hint is built via
`RATING_HINT_TEMPLATE.format(rating_value=...)`. Done as **pure relocation only**
(the `1/2/3/NI/NA` token parameterisation in Β§3 below was deliberately *not*
performed β€” it would break the byte-for-byte requirement; see Β§3 note). Guard:
`backend/tests/test_grounding_prompt_relocation.py` pins SHA-256
`75b4d183…b06c` (len 363) and the hint string.
### 2.3 `backend/core/hybrid_discovery_compiler.py` β€” rating-name default (line ~65)
```python
schema.rating_system = RatingSystem(
detected=True,
name=schema.rating_system.name or "Condition Rating", # <-- hardcoded fallback
type=schema.rating_system.type or "text",
values=rating_values,
...
)
```
- **Match that trips the test:** `"Condition Rating"`.
- **Role:** fallback label for the rating system when discovery found rating
*legends/values* but no explicit *name*.
- **Blast radius:** the user-facing rating label in DOCX headings
(`docx_builder._rating_label`) and previews. Cosmetic but visible.
- **Load-bearing:** PARTIAL (display only; no logic branches on the literal).
- **RESOLVED β†’** `rics_canonical_l3.DEFAULT_RATING_SYSTEM_NAME = "Condition Rating"`,
imported into `hybrid_discovery_compiler` (`name=schema.rating_system.name or
DEFAULT_RATING_SYSTEM_NAME`). **Value preserved exactly.** Note this fallback
(`"Condition Rating"`) is intentionally distinct from the canonical schema's
fully-qualified `rating_system.name` (`"RICS Condition Rating"`, line ~284) β€”
the two were **not** collapsed, to avoid changing a value.
---
## 3. Target schema-driven shape
The relevant model already exists β€” `backend/models/schema.py::RatingSystem`:
```python
class RatingSystem(BaseModel):
detected: bool = False
name: str = "" # discovered label from the template
type: str | None = None
values: list[RatingValue] = [] # discovered legend (e.g. 1/2/3/NI/NA)
format_template: str | None = None
inline_example: str | None = None
```
Intended end-state per violation:
| # | Module | Move the literal to | Mechanism |
|---|--------|---------------------|-----------|
| 2.1 | `pii_scrubber._STATUS_TERMS` | schema/canonical-derived whitelist | Build the status-term whitelist from `schema.rating_system.name` + `RatingValue` labels (+ a small canonical seed in `rics_canonical_l3.py`). Whitelist becomes data, not an inline literal. |
| 2.2 | `reference_mapper._RICS_DOMAIN_RULES` | `backend/prompts/` (exempt) | Relocate the prompt fragment to a `prompts/` builder; parameterise allowed rating tokens from `schema.rating_system.values` instead of the hardcoded `1/2/3/NI/NA`. |
| 2.3 | `hybrid_discovery_compiler` default name | `rics_canonical_l3.py` (exempt) | Replace the `or "Condition Rating"` literal with a canonical default constant imported from `rics_canonical_l3`, or leave `name` empty and let `_rating_label` fall back to the canonical default. |
Net effect: the only files that mention "Condition Rating" become the two the
test already exempts (`rics_canonical_l3.py`, `backend/prompts/*`), so the engine
core is genuinely template-agnostic and the test passes without weakening it.
**Deviation from the original mechanism column (as built):**
- **2.2** was executed as **pure verbatim relocation**. The proposed
"parameterise allowed rating tokens from `schema.rating_system.values`" was
deliberately **not** done β€” it would alter the prompt string and break the
byte-for-byte / SHA-256 guard. Token parameterisation remains a *separate,
future* enhancement (it changes behaviour and must be specced/tested on its own).
- **2.1** uses the **canonical seed** (`DEFAULT_RATING_SYSTEM_NAME`) rather than a
live `schema.rating_system.name` lookup, because `PROPTECH_SAFE_WHITELIST` is a
module-level constant with no per-request schema in scope. Net set is identical.
---
## 4. Safe refactor ordering (when undertaken separately)
1. **2.3 first** (lowest risk, display-only). Verify DOCX rating headings still
render the expected label for a RICS-L3 template and for a template with an
explicit custom rating name.
2. **2.2 next** (prompt relocation + parameterisation). Verify with a live/mock
mapping run that allowed rating tokens still match `schema.rating_system.values`;
confirm `test_minimum_weave` / `test_maximum_compose` / `test_medium_expand`.
3. **2.1 last** (highest risk β€” PII). Build the whitelist from schema + canonical
seed; verify reference ingest does **not** redact "condition rating" and that
`assert_no_pii` still blocks genuine PII. Re-run the full PII suite and the
reference-ingest E2E (`test_section_photos`, `test_source_attribution_e2e`).
Each step is independently shippable and independently revertible. Do **not**
batch 2.1 with 2.2 β€” PII and prompt-contract regressions must be bisectable.
### Verification gate for the whole refactor
Status: **PASSING** β€” full `backend/tests` suite green (0 failures); the three
`test_no_rics_hardcodes` cases now pass, plus the two new relocation guards.
```
python -m pytest backend/tests/test_no_rics_hardcodes.py \
backend/tests/test_grounding_prompt_relocation.py \
backend/tests/test_pii_whitelist_source.py \
backend/tests/test_pii_scrubber.py \
backend/tests/test_section_photos.py \
backend/tests/test_source_attribution_e2e.py \
backend/tests/test_minimum_weave.py backend/tests/test_maximum_compose.py \
backend/tests/test_medium_expand.py --no-cov
```