Skip to content

feat(address): AddressGenerator(dataset='europe') region-group support#195

Merged
ake2l merged 1 commit into
developmentfrom
feat/address-region-group
Jul 6, 2026
Merged

feat(address): AddressGenerator(dataset='europe') region-group support#195
ake2l merged 1 commit into
developmentfrom
feat/address-region-group

Conversation

@ake2l

@ake2l ake2l commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

  • `dataset=` on `AddressGenerator`/`` can now be a region-group alias (`REGION_GROUPS` in new `region_groups.py`, starting with `"europe"` - geographic Europe, restricted to countries that already have a `city_{CC}.csv` in this repo, no new locale data). Each address independently draws a concrete country from the group.
  • The naive approach (resolve the country once in `AddressGenerator.init`) is wrong: a single `AddressGenerator` instance is reused across every row of a run (traced through `generate_worker.py`/`BaseDomainService.generate()` - a fresh `Address` wrapper is built per row, but the same generator underneath). That would give every row the same country, not variety.
  • `AddressGenerator.resolve_row()` draws a fresh country per call for a region group, lazily building and caching city/country/phone/street sub-generators per country encountered (a repeat draw reuses its already-loaded data). A concrete single dataset resolves to the same cached row every time - zero behavior change on the common path.
  • `Address` resolves its row once in `init` and reads every property through it, so city/phone/street/country_code all agree on the same drawn country for a given address.

Test plan

  • New DSL test: 40 `dataset="europe"` addresses produce >1 distinct country, all within the group; city/phone/street stay consistent with the drawn country; a single-country dataset (`DE`) is completely unaffected; deterministic under a seed.
  • Confirmed the discriminating test actually discriminates: patched `resolve_row()` to cache one row instead of per-call (the naive bug) and confirmed it fails.
  • Updated a hand-rolled `AddressGenerator` test double elsewhere (`test_person_gender_alignment.py`) to the new interface.
  • Full suite 1432 passed, 15 skipped, mypy/ruff clean.

dataset= can now be a region-group alias (REGION_GROUPS in the new
region_groups.py, starting with "europe") that draws a concrete country per
address instead of one fixed country.

The naive approach - resolve the country once in AddressGenerator.__init__ -
is wrong: a single AddressGenerator instance is reused across every row of a
run (BaseDomainService.generate() builds a fresh Address per row but keeps
the same underlying generator, confirmed by tracing generate_worker.py). That
would give every row in the run the same country, not variety.

Instead: AddressGenerator.resolve_row() draws a fresh country per call for a
region group (unchanged, cached single row for a concrete dataset - zero
behavior change on the common path), lazily building and caching city/
country/phone/street sub-generators per country encountered so a repeated
draw reuses its already-loaded data instead of reloading it. Address resolves
its row once in __init__ and reads every property through it, so city/
phone/street/country_code all agree on the same drawn country.

Verified the naive single-resolve bug is exactly what the new discriminating
test (test_europe_draws_vary_per_row_and_stay_in_the_group) catches - patched
resolve_row() to cache one row instead of per-call and confirmed it fails.

Updated a hand-rolled AddressGenerator test double (_SentinelAddressGenerator
in test_person_gender_alignment.py) to the new resolve_row() interface.

Full suite 1432 passed/15 skipped, mypy/ruff clean.
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

@ake2l
ake2l merged commit 45a8984 into development Jul 6, 2026
16 checks passed
ake2l added a commit that referenced this pull request Jul 9, 2026
…memstore aggregation

Three small Benerator-migration fixes surfaced by converting a real enterprise project, each with DSL happy/edge/error coverage.

1. **CSV headers are whitespace-trimmed on read** (`FileUtil.read_csv_to_dict_list` - the single reader all CSV paths funnel through). Padded/aligned CSVs (Benerator entity-CSV style: `ean_code     ,name    ,...`) used to produce keys like `"name    "` that script field access (`this.name`) could not resolve. Only the KEYS are trimmed - cell VALUES keep their whitespace untouched (trimming values would corrupt data; only the keys are structural).

2. **Sub-region dataset groups** for `AddressGenerator`: `western_europe`, `central_europe`, `southern_europe`, `eastern_europe`, `north_america`, `oceania`, `french`, `iberia` - pure data additions over the existing region-group mechanism (#195), no engine change. Every listed code verified to have city/street data files, enforced by a new data gate test (a group member without data would otherwise only fail when the seeded draw happens to pick it - a flaky runtime crash).

3. **`Memstore.sumEntityColumn` skips non-numeric cells** (Benerator-lenient aggregation): a CSV-sourced column carrying a stray placeholder (`'n/a'`, empty) no longer aborts the whole sum. Leniency is strictly per-CELL - a wrong column NAME still raises `KeyError` instead of silently summing to 0.

## Test plan
- [x] Region groups (DSL): `iberia` draws vary per row, stay inside the pool, and city/country stay consistent (happy); uppercase `NORTH_AMERICA` resolves like lowercase (edge); an unknown dataset fails loudly (error). Data gate: every code in every group has city/street CSVs.
- [x] Header trim (DSL, end to end): `this.name` resolves on a padded header; record keys are clean; padded cell values stay untouched (contract boundary). Unit test kept.
- [x] Lenient sum: DSL end-to-end (`5, n/a, 7` sums to 12) + unit edge cases (`None`/`'n/a'` skipped) + missing-column-stays-fatal regression.
- [x] All three behavior tests verified discriminating (fixes reverted locally → exactly those tests fail).
- [x] Full suite 1464 passed / 15 skipped, mypy (424 files) and ruff clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant