From 4da27c38bb3aa58cb7602f29f98d8134dfd12246 Mon Sep 17 00:00:00 2001 From: Kyou Date: Thu, 27 Aug 2026 19:33:09 +0800 Subject: [PATCH] fix(config): honor boolean env overrides in Config.get() (#1038) fix(config): honor boolean env overrides in Config.get() (#1038) Config.get() checked int before bool. Since bool subclasses int, boolean environment values could be ignored or returned as integers. Check bool first and strip whitespace before parsing boolean environment values. This applies to the config modules for conflicts, deduplication, split, embeddings, export, ingest, kg, normalize, ontology, and parse. Also make _load_env_vars() use the same whitespace handling for mapped and generic environment variables. Fixes #1035 --- semantica/conflicts/config.py | 8 +- semantica/deduplication/config.py | 8 +- semantica/embeddings/config.py | 12 +- semantica/export/config.py | 12 +- semantica/ingest/config.py | 16 +-- semantica/kg/config.py | 12 +- semantica/normalize/config.py | 12 +- semantica/ontology/config.py | 12 +- semantica/parse/config.py | 12 +- semantica/split/config.py | 6 +- tests/conflicts/test_config_bool_env.py | 168 ++++++++++++++++++++++++ 11 files changed, 223 insertions(+), 55 deletions(-) create mode 100644 tests/conflicts/test_config_bool_env.py diff --git a/semantica/conflicts/config.py b/semantica/conflicts/config.py index c258a7c5..ec76f650 100644 --- a/semantica/conflicts/config.py +++ b/semantica/conflicts/config.py @@ -109,7 +109,7 @@ class ConflictsConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -136,12 +136,12 @@ class ConflictsConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/deduplication/config.py b/semantica/deduplication/config.py index 3b919135..b7313f8a 100644 --- a/semantica/deduplication/config.py +++ b/semantica/deduplication/config.py @@ -110,7 +110,7 @@ class DeduplicationConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -137,12 +137,12 @@ class DeduplicationConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/embeddings/config.py b/semantica/embeddings/config.py index 7793a043..4311ce3a 100644 --- a/semantica/embeddings/config.py +++ b/semantica/embeddings/config.py @@ -106,7 +106,7 @@ class EmbeddingsConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -123,8 +123,8 @@ class EmbeddingsConfig: if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() # Try to convert to appropriate type - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -149,12 +149,12 @@ class EmbeddingsConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/export/config.py b/semantica/export/config.py index 58680f4c..b6befbf3 100644 --- a/semantica/export/config.py +++ b/semantica/export/config.py @@ -105,7 +105,7 @@ class ExportConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -122,8 +122,8 @@ class ExportConfig: if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() # Try to convert to appropriate type - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -148,12 +148,12 @@ class ExportConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/ingest/config.py b/semantica/ingest/config.py index 17a623a2..e2cfb017 100644 --- a/semantica/ingest/config.py +++ b/semantica/ingest/config.py @@ -110,7 +110,7 @@ class IngestConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -127,8 +127,8 @@ class IngestConfig: if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() # Try to convert to appropriate type - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -143,8 +143,8 @@ class IngestConfig: if key.startswith(mcp_prefix) and key not in env_mappings: config_key = key[len(mcp_prefix) :].lower() # Try to convert to appropriate type - if value.lower() in ("true", "false"): - self._configs[f"mcp_{config_key}"] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[f"mcp_{config_key}"] = value.strip().lower() == "true" elif value.isdigit(): self._configs[f"mcp_{config_key}"] = int(value) else: @@ -169,12 +169,12 @@ class IngestConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/kg/config.py b/semantica/kg/config.py index 0d6c1228..d7f7a065 100644 --- a/semantica/kg/config.py +++ b/semantica/kg/config.py @@ -106,7 +106,7 @@ class KGConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -123,8 +123,8 @@ class KGConfig: if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() # Try to convert to appropriate type - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -149,12 +149,12 @@ class KGConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/normalize/config.py b/semantica/normalize/config.py index e44750fd..7e296769 100644 --- a/semantica/normalize/config.py +++ b/semantica/normalize/config.py @@ -98,7 +98,7 @@ class NormalizeConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -113,8 +113,8 @@ class NormalizeConfig: for key, value in os.environ.items(): if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -136,12 +136,12 @@ class NormalizeConfig: value = os.getenv(env_key) if value: try: - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/ontology/config.py b/semantica/ontology/config.py index 82bc13b7..7fcd6cb6 100644 --- a/semantica/ontology/config.py +++ b/semantica/ontology/config.py @@ -99,7 +99,7 @@ class OntologyConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -114,8 +114,8 @@ class OntologyConfig: for key, value in os.environ.items(): if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -137,12 +137,12 @@ class OntologyConfig: value = os.getenv(env_key) if value: try: - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/parse/config.py b/semantica/parse/config.py index cc919d76..68f93b2f 100644 --- a/semantica/parse/config.py +++ b/semantica/parse/config.py @@ -98,7 +98,7 @@ class ParseConfig: if value: try: if type_func == bool: - self._configs[config_key] = value.lower() in ( + self._configs[config_key] = value.strip().lower() in ( "true", "1", "yes", @@ -113,8 +113,8 @@ class ParseConfig: for key, value in os.environ.items(): if key.startswith(env_prefix) and key not in env_mappings: config_key = key[len(env_prefix) :].lower() - if value.lower() in ("true", "false"): - self._configs[config_key] = value.lower() == "true" + if value.strip().lower() in ("true", "false"): + self._configs[config_key] = value.strip().lower() == "true" elif value.isdigit(): self._configs[config_key] = int(value) else: @@ -136,12 +136,12 @@ class ParseConfig: value = os.getenv(env_key) if value: try: - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/semantica/split/config.py b/semantica/split/config.py index 1dc1cda0..46ecfaa2 100644 --- a/semantica/split/config.py +++ b/semantica/split/config.py @@ -123,12 +123,12 @@ class SplitConfig: if value: try: # Try to convert to appropriate type - if isinstance(default, int): + if isinstance(default, bool): + return value.strip().lower() in ("true", "1", "yes", "on") + elif isinstance(default, int): return int(value) elif isinstance(default, float): return float(value) - elif isinstance(default, bool): - return value.lower() in ("true", "1", "yes", "on") return value except (ValueError, TypeError): pass diff --git a/tests/conflicts/test_config_bool_env.py b/tests/conflicts/test_config_bool_env.py new file mode 100644 index 00000000..e78dd378 --- /dev/null +++ b/tests/conflicts/test_config_bool_env.py @@ -0,0 +1,168 @@ +'''Regression tests for issue #1035. + +Config.get() must honor boolean environment overrides. Previously the type +dispatch checked isinstance(default, int) before isinstance(default, bool); +since bool subclasses int the bool branch was unreachable, so a boolean +default silently ignored the environment variable (or returned an int when the +value happened to parse). +''' + +import os +import unittest +from unittest.mock import patch + +from semantica.conflicts.config import ConflictsConfig +from semantica.deduplication.config import DeduplicationConfig +from semantica.embeddings.config import EmbeddingsConfig +from semantica.export.config import ExportConfig +from semantica.ingest.config import IngestConfig +from semantica.kg.config import KGConfig +from semantica.normalize.config import NormalizeConfig +from semantica.ontology.config import OntologyConfig +from semantica.parse.config import ParseConfig +from semantica.split.config import SplitConfig + + +# The three modules whose get() reaches the type dispatch (no generic +# PREFIX_* scanner in _load_env_vars pre-populates _configs). +_DISPATCH_MODULES = [ + ("conflicts", "CONFLICT", ConflictsConfig), + ("deduplication", "DEDUP", DeduplicationConfig), + ("split", "SPLIT", SplitConfig), +] + + +class TestConfigBoolEnvOverride(unittest.TestCase): + """Boolean env overrides must reach Config.get() on the dispatch path.""" + + def test_conflicts_bool_true(self): + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "true"}): + self.assertIs(ConflictsConfig().get("zztestflag", False), True) + + def test_conflicts_bool_false(self): + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "false"}): + self.assertIs(ConflictsConfig().get("zztestflag", True), False) + + def test_conflicts_int_still_parsed(self): + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "42"}): + self.assertEqual(ConflictsConfig().get("zztestflag", 0), 42) + + def test_conflicts_float_still_parsed(self): + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "1.5"}): + self.assertEqual(ConflictsConfig().get("zztestflag", 0.0), 1.5) + + def test_conflicts_str_still_returned(self): + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "some-value"}): + self.assertEqual(ConflictsConfig().get("zztestflag", "x"), "some-value") + + def test_truthy_spellings(self): + for value in ["true", "1", "yes", "on", "TRUE", "True", "YES", "ON", " true "]: + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": value}): + self.assertIs( + ConflictsConfig().get("zztestflag", False), + True, + msg="value {0!r} should parse as True".format(value), + ) + + def test_falsy_spellings(self): + for value in ["false", "0", "no", "off", "FALSE", "False", "NO", "OFF", " false "]: + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": value}): + self.assertIs( + ConflictsConfig().get("zztestflag", True), + False, + msg="value {0!r} should parse as False".format(value), + ) + + def test_programmatic_config_takes_precedence(self): + cfg = ConflictsConfig() + cfg.set("zztestflag", "nope") + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "true"}): + self.assertEqual(cfg.get("zztestflag", False), "nope") + + def test_non_bool_value_falls_back_to_default(self): + # "not-a-number" is not in the truthy set -> False for a bool default; + # the int branch would have raised and swallowed the env var entirely. + with patch.dict(os.environ, {"CONFLICT_ZZTESTFLAG": "not-a-number"}): + self.assertIs(ConflictsConfig().get("zztestflag", True), False) + + def test_bool_dispatch_across_three_modules(self): + for name, prefix, cls in _DISPATCH_MODULES: + env_key = prefix + "_ZZTESTFLAG" + with self.subTest(module=name, env_key=env_key): + with patch.dict(os.environ, {env_key: "true"}): + self.assertIs(cls().get("zztestflag", False), True) + with patch.dict(os.environ, {env_key: "false"}): + self.assertIs(cls().get("zztestflag", True), False) + with patch.dict(os.environ, {env_key: "1"}): + self.assertIs(cls().get("zztestflag", False), True) + with patch.dict(os.environ, {env_key: "0"}): + self.assertIs(cls().get("zztestflag", True), False) + + +# Every module config also maps known boolean env vars in _load_env_vars; +# those must parse as real bools too (not ints or strings). +_MAPPED_BOOL_ENV = [ + ("conflicts", "CONFLICT_AUTO_RESOLVE", ConflictsConfig), + ("deduplication", "DEDUP_USE_CLUSTERING", DeduplicationConfig), + ("embeddings", "EMBEDDING_NORMALIZE", EmbeddingsConfig), + ("export", "EXPORT_VALIDATE", ExportConfig), + ("ingest", "INGEST_RECURSIVE", IngestConfig), + ("kg", "KG_MERGE_ENTITIES", KGConfig), + ("ontology", "ONTOLOGY_CHECK_CONSISTENCY", OntologyConfig), + ("parse", "PARSE_EXTRACT_TABLES", ParseConfig), +] + +# Modules that only reach bool env parsing via the generic PREFIX_* scanner +# in _load_env_vars (no bool entry in env_mappings). +_SCANNER_BOOL_ENV = [ + ("split", "SPLIT_ZZTESTFLAG", SplitConfig), + ("normalize", "NORMALIZE_ZZTESTFLAG", NormalizeConfig), +] + + +class TestMappedBoolEnvVars(unittest.TestCase): + def test_mapped_bool_true(self): + for module, env_key, cls in _MAPPED_BOOL_ENV: + with self.subTest(module=module, env_key=env_key): + with patch.dict(os.environ, {env_key: "true"}): + cfg = cls() + key = env_key.split("_", 1)[1].lower() + self.assertIs(cfg.get(key, False), True) + + def test_mapped_bool_false(self): + for module, env_key, cls in _MAPPED_BOOL_ENV: + with self.subTest(module=module, env_key=env_key): + with patch.dict(os.environ, {env_key: "false"}): + cfg = cls() + key = env_key.split("_", 1)[1].lower() + self.assertIs(cfg.get(key, True), False) + + def test_mapped_bool_with_whitespace(self): + # Qodo follow-up: _load_env_vars() must strip like get() does, so a + # padded value (" true ") is not silently parsed as False. + for module, env_key, cls in _MAPPED_BOOL_ENV: + with self.subTest(module=module, env_key=env_key): + with patch.dict(os.environ, {env_key: " true "}): + cfg = cls() + key = env_key.split("_", 1)[1].lower() + self.assertIs(cfg.get(key, False), True) + + def test_scanner_bool_true(self): + for module, env_key, cls in _SCANNER_BOOL_ENV: + with self.subTest(module=module, env_key=env_key): + with patch.dict(os.environ, {env_key: "true"}): + cfg = cls() + key = env_key.split("_", 1)[1].lower() + self.assertIs(cfg.get(key, False), True) + + def test_scanner_bool_with_whitespace(self): + for module, env_key, cls in _SCANNER_BOOL_ENV: + with self.subTest(module=module, env_key=env_key): + with patch.dict(os.environ, {env_key: " true "}): + cfg = cls() + key = env_key.split("_", 1)[1].lower() + self.assertIs(cfg.get(key, False), True) + + +if __name__ == "__main__": + unittest.main()