#131 update focus validator to support focus 1.3 requirements modelling and entity type Object.#132
Conversation
…and entity type Object. Signed-off-by: Mike Fuller <mike@finops.org>
Signed-off-by: Mike Fuller <mike@finops.org>
ce54958 to
7ad49d7
Compare
Signed-off-by: Mike Fuller <mike@finops.org>
|
Ready for your review @Matt-Cowsert, after which we can move to release v2.1.0 |
There was a problem hiding this comment.
The generator architecture for the JSON checks makes sense — consistent pattern across array and single-value paths, and using SkippedMissingGeneratorCheck instead of crashing on unknown check functions feels like the right call for forward compatibility.
Version bump and outputter_web.py changes look clean.
Non-blocking comments
1. Inconsistent brace patterns in SQL templates
Some generators use {{table_name}} (double braces in the f-string) and others use {{{{table_name}}}} (quadruple). Both work because the downstream .replace() on lines 4890-4891 and 5986-5987 handles both patterns, but the inconsistency could trip up future contributors. Worth standardizing on one pattern.
Quadruple-brace generators: JSONCheckPathNumericFormatGenerator, JSONCheckPathUnitFormatGenerator, JSONCheckPathDistinctParentGenerator, FormatJSONFormatGenerator (8 occurrences total).
2. _generate_unit_format_regex duplicated across classes
The ~50-line unit format regex builder is copy-pasted identically in JSONCheckPathUnitFormatGenerator and JSONFormatUnitGenerator (and presumably the existing FormatUnitGenerator). If the FOCUS unit spec evolves, three places need updating. Consider extracting to a module-level function.
3. Near-duplicate generator pairs
JSONFormatNumericGenerator / JSONCheckPathNumericFormatGenerator and JSONFormatUnitGenerator / JSONCheckPathUnitFormatGenerator are structurally identical. A shared base class could cut ~400 lines.
4. No tests for the 14 new generators
The JSON path validation logic (array vs. single, null handling, type detection) has enough edge cases that unit tests would add real value. Not blocking for initial merge, but worth tracking.
Signed-off-by: Mike Fuller mike@finops.org