Skip to content

Commit 5cf7463

Browse files
fix: reject out-of-range profiler tunables instead of self-contradicting
min_share / intersection_floor / missing_flag / reference_flag were accepted as any float, and imbalance_flag / min_group_size as any number, with no range check anywhere in the CLI -> _build_opts -> _resolve_opts chain. min_share=1.5 (a percentage/fraction typo) was silently accepted: every group got flagged "under-represented" in flags while overall_score/grade stayed 100/"A" - the two halves of the same report contradicting each other with no error. _resolve_opts (and its JS mirror resolveOpts) now validate: - min_share, intersection_floor, missing_flag, reference_flag in [0, 1] - imbalance_flag >= 1 - min_group_size >= 1 raising ValueError, which the CLI already renders as "error: ..." (exit 2) and the MCP server wraps as a ToolError. Covers profile_dataset, compare_datasets, `faircode profile`, and `faircode compare`. Closes #511 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 351a2d3 commit 5cf7463

5 files changed

Lines changed: 92 additions & 0 deletions

File tree

assets/profiler-engine.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,28 @@
3636
reference: null // {column: {group: expected_share}} baseline (SPEC 8)
3737
};
3838

39+
// SPEC section 7 tunables that must fall in [0, 1]. Mirrors
40+
// faircode.profiler._UNIT_INTERVAL_OPTS.
41+
var UNIT_INTERVAL_OPTS = ['min_share', 'intersection_floor', 'missing_flag', 'reference_flag'];
42+
43+
function validateOpts(o) {
44+
// Reject out-of-range tunables instead of silently producing a
45+
// self-contradictory report (#511). Mirrors _validate_opts in the
46+
// Python engine.
47+
UNIT_INTERVAL_OPTS.forEach(function (k) {
48+
var v = o[k];
49+
if (v !== null && v !== undefined && !(v >= 0 && v <= 1)) {
50+
throw new Error(k + ' must be between 0 and 1, got ' + v);
51+
}
52+
});
53+
if (o.imbalance_flag !== null && o.imbalance_flag !== undefined && o.imbalance_flag < 1) {
54+
throw new Error('imbalance_flag must be >= 1, got ' + o.imbalance_flag);
55+
}
56+
if (o.min_group_size !== null && o.min_group_size !== undefined && o.min_group_size < 1) {
57+
throw new Error('min_group_size must be >= 1, got ' + o.min_group_size);
58+
}
59+
}
60+
3961
function resolveOpts(opts) {
4062
var o = {};
4163
Object.keys(DEFAULT_OPTS).forEach(function (k) { o[k] = DEFAULT_OPTS[k]; });
@@ -44,6 +66,7 @@
4466
if (opts[k] !== null && opts[k] !== undefined) o[k] = opts[k];
4567
});
4668
}
69+
validateOpts(o);
4770
return o;
4871
}
4972
// Comparison / drift (SPEC section 8)

faircode/profiler.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,31 @@ def _wilson(count: int, n: int) -> tuple:
6565
}
6666

6767

68+
_UNIT_INTERVAL_OPTS = ("min_share", "intersection_floor", "missing_flag", "reference_flag")
69+
70+
71+
def _validate_opts(o: dict) -> None:
72+
"""Reject out-of-range tunables (SPEC section 7) instead of silently
73+
producing a self-contradictory report - e.g. min_share=1.5 flags every
74+
group as under-represented while overall_score/grade stay 100/"A" (#511).
75+
"""
76+
for key in _UNIT_INTERVAL_OPTS:
77+
v = o.get(key)
78+
if v is not None and not 0.0 <= v <= 1.0:
79+
raise ValueError(f"{key} must be between 0 and 1, got {v!r}")
80+
imbalance = o.get("imbalance_flag")
81+
if imbalance is not None and imbalance < 1.0:
82+
raise ValueError(f"imbalance_flag must be >= 1, got {imbalance!r}")
83+
min_group_size = o.get("min_group_size")
84+
if min_group_size is not None and min_group_size < 1:
85+
raise ValueError(f"min_group_size must be >= 1, got {min_group_size!r}")
86+
87+
6888
def _resolve_opts(opts) -> dict:
6989
o = dict(_DEFAULT_OPTS)
7090
if opts:
7191
o.update({k: v for k, v in opts.items() if v is not None})
92+
_validate_opts(o)
7293
return o
7394

7495

tests/test_cli.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,19 @@ def test_profile_fail_under_returns_nonzero_and_explains_score(tmp_path, capsys)
3636
assert "representation score 72/100 is below --fail-under 90" in captured.err
3737

3838

39+
def test_profile_rejects_out_of_range_min_share(tmp_path, capsys):
40+
# A percentage/fraction typo (15 instead of 0.15, or 1.5) used to be
41+
# accepted silently and produce a self-contradictory report (#511).
42+
path = tmp_path / "a.csv"
43+
path.write_text("sex\n" + "M\n" * 50 + "F\n" * 50, encoding="utf-8")
44+
45+
exit_code = main(["profile", str(path), "--min-share", "1.5"])
46+
47+
captured = capsys.readouterr()
48+
assert exit_code != 0
49+
assert "min_share must be between 0 and 1" in captured.err
50+
51+
3952
def test_profile_fail_under_keeps_json_output_machine_readable(tmp_path, capsys):
4053
path = tmp_path / "balanced.csv"
4154
path.write_text("sex\nM\nF\nM\nF\n", encoding="utf-8")

tests/test_js_parity.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,25 @@ def test_python_js_profiler_parity_with_overrides_cross_and_thresholds(tmp_path)
234234
assert any("reference" in d for d in python_result["dimensions"])
235235

236236

237+
def test_python_js_reject_out_of_range_min_share_parity(tmp_path):
238+
"""Both engines reject an out-of-range tunable (min_share=1.5) rather
239+
than silently producing a self-contradictory report (#511)."""
240+
csv = CSV_PATHS["small.csv"]
241+
242+
with pytest.raises(ValueError, match="min_share must be between 0 and 1"):
243+
profile(pd.read_csv(csv), opts={"min_share": 1.5})
244+
245+
opts_path = tmp_path / "opts.json"
246+
opts_path.write_text(json.dumps({"opts": {"min_share": 1.5}}), encoding="utf-8")
247+
248+
completed = subprocess.run(
249+
["node", "scripts/engine-js.js", "profile", str(csv), str(opts_path)],
250+
capture_output=True, text=True, encoding="utf-8", check=False,
251+
)
252+
assert completed.returncode != 0
253+
assert "min_share must be between 0 and 1" in completed.stderr
254+
255+
237256
def test_python_js_cross_parity_on_unmatched_column(tmp_path):
238257
"""An unmatched `cross` column raises the same error on both engines
239258
instead of the JS engine silently falling back to the first two detected

tests/test_profiler.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,22 @@ def test_imbalance_flag_tunable():
308308
for f in profile(df, opts={"imbalance_flag": 5.0})["flags"])
309309

310310

311+
@pytest.mark.parametrize("opts, message", [
312+
({"min_share": 1.5}, "min_share must be between 0 and 1"),
313+
({"min_share": -0.1}, "min_share must be between 0 and 1"),
314+
({"intersection_floor": 2.0}, "intersection_floor must be between 0 and 1"),
315+
({"missing_flag": 5.0}, "missing_flag must be between 0 and 1"),
316+
({"imbalance_flag": 0.5}, "imbalance_flag must be >= 1"),
317+
({"min_group_size": 0}, "min_group_size must be >= 1"),
318+
])
319+
def test_out_of_range_tunables_raise_instead_of_contradicting_themselves(opts, message):
320+
# min_share=1.5 used to be accepted silently: every group flagged
321+
# "under-represented" while overall_score/grade stayed 100/"A" (#511).
322+
df = pd.DataFrame({"sex": ["M"] * 50 + ["F"] * 50})
323+
with pytest.raises(ValueError, match=message):
324+
profile(df, opts=opts)
325+
326+
311327
# ── Choosable intersection pair (issue #58) ──────────────────────────────────
312328
def test_cross_selects_intersection_pair():
313329
df = pd.DataFrame({

0 commit comments

Comments
 (0)