Add the check the writer cannot make about itself - it found 17 wrong rates
Asked whether this needs AI or whether code can do it. The answer turned out to
be neither, exactly: what was missing was not a better matcher, it was a check
the matcher could not fake.
Every existing verification asks 'did the value land in the cell I aimed at?' -
and the writer answers that for itself. So a confidently misaligned write sails
straight through. That is the hole all three of the last bugs went through.
populate() now records which line it placed on which row, and verify() re-reads
that row out of the written file and asks whether it really is that line. The
test is CONTAINMENT, not similarity: a line split across two rows legitimately
puts only a fragment in the cell, and a similarity cut-off flagged every one of
those as an error. Case, double spaces and the row's own leading label are
normalised first.
Verified to actually fail when it should: re-introducing the repeated-header
bug and the split-row bug both trip it, and it stays quiet on a clean build.
It pays for itself immediately. Against Central Towers it finds 18 misplaced
lines, 17 of which had a rate written into the wrong row - the aligner drifts
one or two rows inside blocks of near-identical lines ('.02 Over 2,0 m up to
3,0 m' / '.03 Over 3,0 m up to 4,0 m'). Nothing else could see it: counts
were healthy, every write landed, every formula was intact, the total
reconciled, and 17 rates were still priced against the wrong descriptions.
The check is FATAL, so the download is removed while it is outstanding.
Blueberry passes and is unaffected. Central Towers is now correctly BLOCKED -
a file that refuses to download beats one that quietly misprices 17 lines.
The underlying alignment drift is NOT fixed yet. That is the next piece of
work, and it is better done measured end to end than patched line by line.
This commit is contained in:
parent
1228ad061e
commit
19918c9a3e
3 changed files with 149 additions and 1 deletions
31
README.md
31
README.md
|
|
@ -347,6 +347,37 @@ every such line with its best five candidate rows, already ranked and scored:
|
|||
Pick one, or leave it unplaced to keep it out of the workbook. Chosen lines are
|
||||
written on the next run and the same verification gate applies to the result.
|
||||
|
||||
## The check the writer could not make about itself
|
||||
|
||||
Every other verification asks *"did the value land in the cell I aimed at?"* —
|
||||
and that is a question the **writer** answers for itself. So a confidently
|
||||
misaligned write passes it perfectly: a rate on `DESCRIPTION: CENTRAL TOWERS`,
|
||||
one line's rate on its neighbour's row, all invisible.
|
||||
|
||||
`populate()` now records where it placed each line, and the verifier re-reads
|
||||
that row out of the written file and asks whether it really is that line. The
|
||||
test is **containment**, not similarity — a line split across two rows
|
||||
legitimately puts only a fragment in the cell, and a similarity cut-off
|
||||
flagged those as errors.
|
||||
|
||||
It pays for itself immediately. Run against Central Towers it found **18
|
||||
misplaced lines, 17 of which had a rate written into the wrong row** — the
|
||||
aligner drifts one or two rows inside blocks of near-identical lines like
|
||||
|
||||
```
|
||||
.02 Over 2,0 m up to 3,0 m
|
||||
.03 Over 3,0 m up to 4,0 m
|
||||
```
|
||||
|
||||
Nothing else could see this. Counts were healthy, every write "landed", every
|
||||
formula was intact, and the total reconciled — and 17 rates were still on the
|
||||
wrong lines.
|
||||
|
||||
**The check is fatal, so the download is removed while this is outstanding.**
|
||||
Blueberry passes and is unaffected. Central Towers is currently blocked, which
|
||||
is correct: a file that refuses to download is better than one that quietly
|
||||
prices 17 lines against the wrong descriptions.
|
||||
|
||||
## The download is gated
|
||||
|
||||
Nothing is offered for download until the **output file has been read back off
|
||||
|
|
|
|||
|
|
@ -573,6 +573,9 @@ class Result:
|
|||
# verifier can read the file back and check each one rather than trusting
|
||||
# that a write landed where we think it did
|
||||
written_cells: list = field(default_factory=list)
|
||||
# (sheet, row, line) for every line the writer placed - the ground truth
|
||||
# the placement check verifies against
|
||||
placements: list = field(default_factory=list)
|
||||
findings: list[Finding] = field(default_factory=list)
|
||||
sheets: list[str] = field(default_factory=list)
|
||||
layout_notes: list[str] = field(default_factory=list)
|
||||
|
|
@ -580,6 +583,24 @@ class Result:
|
|||
def wrote(self, sheet: str, row: int, col: int, value) -> None:
|
||||
self.written_cells.append((sheet, row, col, value))
|
||||
|
||||
def placed(self, sheet: str, row: int, ln: dict, slot_desc: str = "") -> None:
|
||||
"""Record that this line was placed on this row.
|
||||
|
||||
The writer is the only thing that knows this for certain, so it says so
|
||||
explicitly rather than the verifier trying to reverse-engineer it from
|
||||
the cell values afterwards.
|
||||
"""
|
||||
self.placements.append({
|
||||
"sheet": sheet, "row": row,
|
||||
"number": ln.get("number", ""), "desc": ln.get("desc", ""),
|
||||
"unit": ln.get("unit", ""), "qty": ln.get("qty"),
|
||||
"rate": ln.get("rate"),
|
||||
# the slot's own merged description - for a line split across two
|
||||
# rows this is the full text, where the source line may be cut off
|
||||
# by the PDF's own wrapping
|
||||
"slot_desc": slot_desc,
|
||||
})
|
||||
|
||||
def add(self, *a, **kw) -> None:
|
||||
self.findings.append(Finding(*a, **kw))
|
||||
|
||||
|
|
@ -948,6 +969,11 @@ def _write_one(wb, name: str, lay: Layout, ln: dict,
|
|||
|
||||
ws = wb[name]
|
||||
r = slot.row
|
||||
# The writer is the only thing that knows which line went on which row, so
|
||||
# it records that explicitly. The verification then re-reads the row and
|
||||
# asks whether it really is that line - which is the question the writer
|
||||
# cannot answer about itself.
|
||||
res.placed(name, r, ln, slot.desc or "")
|
||||
c_unit, c_qty = lay.col("unit"), lay.col("qty")
|
||||
c_rate, c_amt = lay.col("rate"), lay.col("amount")
|
||||
label = f"{slot.number} {slot.desc}"[:70]
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ blocking the run.
|
|||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
|
|
@ -129,6 +130,86 @@ def _summary_total(wb):
|
|||
return None
|
||||
|
||||
|
||||
def _norm_txt(t: str) -> str:
|
||||
"""Case, double spaces and stray punctuation are noise. "b) Temporary R1
|
||||
and W302 signs" and "b) Temporary R1 and W302 signs" are the same line."""
|
||||
return re.sub(r"\s+", " ", (t or "")).strip().lower()
|
||||
|
||||
|
||||
# a leading reference or lettered label on the row itself: ".04 ", "1.2 ",
|
||||
# "c) ", "110.03.01 "
|
||||
_LEAD_LABEL = re.compile(r"^(?:\(?[a-z]\)|\.?\d+(?:\.\d+)*\.?)\s+", re.I)
|
||||
|
||||
|
||||
def _placement_report(result, out) -> list:
|
||||
"""Does every written row actually BELONG to the line written on it?
|
||||
|
||||
The other checks ask "did the value land in the cell I aimed at". That
|
||||
question is answered by the writer itself, so a confidently misaligned
|
||||
write sails straight through: a rate on "DESCRIPTION: CENTRAL TOWERS", or
|
||||
one line's rate sitting on its neighbour's row, both look perfect.
|
||||
|
||||
This asks what the writer cannot check for itself - re-read the row's own
|
||||
description out of the written file and ask whether it really is the line
|
||||
that was placed there. It needs no knowledge of the template, so it holds
|
||||
for every consultant's workbook, present and future.
|
||||
"""
|
||||
from boq_engine.template import _row_desc, detect_layout
|
||||
|
||||
layouts = {}
|
||||
for name in out.sheetnames:
|
||||
try:
|
||||
layouts[name] = detect_layout(out[name])
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
bad, seen_rows = [], {}
|
||||
for pl in getattr(result, "placements", []):
|
||||
sheet, row, want = pl["sheet"], pl["row"], (pl["desc"] or "").strip()
|
||||
if sheet not in out.sheetnames or not want:
|
||||
continue
|
||||
lay = layouts.get(sheet)
|
||||
if lay is None:
|
||||
continue
|
||||
key = (sheet, row)
|
||||
if key in seen_rows and seen_rows[key] != want[:40]:
|
||||
bad.append((sheet, row, want[:44],
|
||||
f"already claimed by {seen_rows[key]!r}"))
|
||||
continue
|
||||
seen_rows[key] = want[:40]
|
||||
|
||||
row_desc = _norm_txt(_row_desc(out[sheet], row, lay))
|
||||
want = _norm_txt(want)
|
||||
if not row_desc:
|
||||
bad.append((sheet, row, want[:44], "that row carries no text at all"))
|
||||
continue
|
||||
# CONTAINMENT, not similarity. A line is often split across rows, so the
|
||||
# row legitimately holds only a fragment of it - "charges" is genuinely
|
||||
# part of "Value-related preliminary and general charges". Testing
|
||||
# containment asks the right question exactly and has no threshold to
|
||||
# tune. A similarity cut-off flagged those legitimate fragments.
|
||||
# the row's own text may start with its own label - ".04 " or "c) " -
|
||||
# which the source line does not carry. Strip it before comparing.
|
||||
rl = _LEAD_LABEL.sub("", row_desc).strip()
|
||||
wl = want
|
||||
# accept a match against the slot's merged text too: a line split
|
||||
# across two rows puts only a fragment in the cell, and the source
|
||||
# line itself may be cut short by the PDF's wrapping
|
||||
sl = _norm_txt(pl.get("slot_desc") or "")
|
||||
# The slot's merged text is only a legitimate match when the row holds
|
||||
# a FRAGMENT of it - a line split across two rows puts "110.04.01
|
||||
# above" in the cell while the merged text is the whole sentence. When
|
||||
# the two are the same length the row IS the whole slot, and matching
|
||||
# it against itself would let any row validate itself.
|
||||
frag = bool(sl) and len(sl) > len(rl) * 1.5
|
||||
def ok(x, host):
|
||||
return bool(x) and x in host
|
||||
if not (ok(rl, wl) or ok(wl, rl) or (frag and ok(rl, sl))):
|
||||
bad.append((sheet, row, want[:44],
|
||||
f"the row reads {row_desc[:44]!r}"))
|
||||
return bad
|
||||
|
||||
|
||||
def verify(src_xlsx: str, out_xlsx: str, tree: dict, result,
|
||||
do_recalc: bool = True) -> Verdict:
|
||||
v = Verdict()
|
||||
|
|
@ -189,7 +270,17 @@ def verify(src_xlsx: str, out_xlsx: str, tree: dict, result,
|
|||
+ (f": {wrong[:4]}" if wrong else ""),
|
||||
fatal=True)
|
||||
|
||||
# ---- 4. COMPLETE: every source line accounted for ----------------------
|
||||
# ---- 4. PLACEMENT: is each written row really that line? ----------------
|
||||
# FATAL. A rate on the wrong line is invisible in the file and expensive in
|
||||
# a tender, which makes it strictly worse than the line being left blank.
|
||||
misplaced = _placement_report(result, out)
|
||||
detail = (f"re-read the written rows and compared each one to the line "
|
||||
f"placed on it; {len(misplaced)} row(s) disagree")
|
||||
for sh, rw, want, why in misplaced[:6]:
|
||||
detail += f"\n {sh} r{rw}: wanted {want!r} but the row says {why}"
|
||||
v.add("every rate sits on the line it belongs to", not misplaced, detail)
|
||||
|
||||
# ---- 5. COMPLETE: every source line accounted for ----------------------
|
||||
unmapped = [f for f in result.findings if f.kind == "unmapped"]
|
||||
v.add("every source line accounted for", not unmapped,
|
||||
f"{len(lines) - len(unmapped)} of {len(lines)} lines placed; "
|
||||
|
|
|
|||
Loading…
Reference in a new issue