File size: 9,678 Bytes
aad7814
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
# 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
```