fix: validate augmented entities before finalization#197
Conversation
|
All contributors have signed the DCO ✍️ ✅ |
|
I have read the DCO document and I hereby sign the DCO. |
Greptile SummaryThis PR closes a security gap where augmented entities produced by the LLM augmenter could reach the final detected-entity set without ever being validated. The previous pipeline only ran LLM validation on seed (GLiNER) entities; after the seed+augmented merge, no second validation was performed before finalization.
Confidence Score: 5/5Safe to merge; the new merged validation pass is correctly wired into the DAG and well-covered by regression tests. The structural change is straightforward: two new columns inserted between MERGE_AND_BUILD_CANDIDATES and APPLY_VALIDATION_AND_FINALIZE, using the existing ChunkedValidation machinery generalised with configurable column parameters. The generalisation is backward-compatible (defaults match the original hardcoded values). All three changed test files have dedicated coverage for the new paths, and existing tests were updated correctly. No files require special attention; both comments are minor documentation and test-assertion nits. Important Files Changed
|
| params = ChunkedValidationParams( | ||
| pool=["v0"], | ||
| max_entities_per_call=10, | ||
| excerpt_window_chars=100, | ||
| entities_column=COL_MERGED_ENTITIES, | ||
| candidates_column=COL_VALIDATION_CANDIDATES, | ||
| prompt_template=_MINIMAL_TEMPLATE, | ||
| ) | ||
|
|
||
| out = chunked_validate_row(row, params, {"v0": facade}) | ||
|
|
||
| decisions = {d["id"]: d["decision"] for d in out[COL_VALIDATION_DECISIONS]["decisions"]} | ||
| assert decisions == {"first_name_0_5": "keep", "api_key_13_29": "drop"} |
There was a problem hiding this comment.
output_column not exercised in the new parameterization test
The test creates ChunkedValidationParams without setting output_column, so the result lands in COL_VALIDATION_DECISIONS (the default). In production, ChunkedValidationGenerator._params() sets output_column=self.config.name, which for the merged pass resolves to COL_MERGED_VALIDATION_DECISIONS. The new test therefore doesn't cover the path where the output is redirected to a non-default column — if the output_column wiring broke, this test would still pass. Adding output_column=COL_MERGED_VALIDATION_DECISIONS to the params and asserting on out[COL_MERGED_VALIDATION_DECISIONS] would close that gap.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Why
The detection workflow only validated seed detector entities before augmentation. After the merged entity set was built, augmented-only entities had no matching validation decisions, and finalization kept unmatched entities unchanged. That allowed hallucinated augmented entities to become final detections without validation.
Impact
Augmented entities now need a validation decision from the merged candidate pass before they can reach the final detected entity set.
Validation
uv run --group dev ruff check src/anonymizer/engine/constants.py src/anonymizer/engine/detection/chunked_validation.py src/anonymizer/engine/detection/custom_columns.py src/anonymizer/engine/detection/detection_workflow.py src/anonymizer/engine/workflow_columns/detection/config.py src/anonymizer/engine/workflow_columns/detection/impl.py tests/engine/test_chunked_validation.py tests/engine/test_detection_custom_columns.py tests/engine/test_detection_workflow.pyuv run --group dev pytest tests/engine/test_chunked_validation.py tests/engine/test_detection_custom_columns.py tests/engine/test_detection_workflow.py -q