From 8ba3a1d0a7ce3a31d899200ee510d16e20e63455 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 17 Apr 2026 23:27:44 +0200 Subject: [PATCH] fix(tooling): address PR #130 review findings (#833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hoshe: - Mark --dump-prompts / name_feature() as vestigial with TODO note - Fix --refresh help string: 200 → 1000 (matches actual default) - Fix _RIVER_POOLS comment numbering: Pool 6 before Pool 5 → correct order - Remove dead first-pass code in fix_fewshot_bleed.py - _CAPTURE_FILE leak noted in vestigial TODO Tyre: - Fix stale "Gemma 2" strings in banner, argparse description, model help - Note dead code for cleanup pass (name_feature ~700 lines) Hoshe (prune): - prune_atlas_features.py: named features sort before unnamed, preventing silent discard of hand-authored names during pruning naming_core: - v0.2: few-shot blocklist, stricter is_valid_name (min 3 chars, no digits, no brackets), prompt fragment rejection expanded Miri clarification: the 261 "empty-string" files contain only roads (37) and railroads (37) — infrastructure features never in naming scope. All cities/rivers/oceans/mountains/POIs are clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- tooling/planet-gen/fix_fewshot_bleed.py | 42 +--------------------- tooling/planet-gen/gemma_naming.py | 25 ++++++------- tooling/planet-gen/prune_atlas_features.py | 10 +++--- 3 files changed, 18 insertions(+), 59 deletions(-) diff --git a/tooling/planet-gen/fix_fewshot_bleed.py b/tooling/planet-gen/fix_fewshot_bleed.py index 4b0851a7a..36ea080bf 100644 --- a/tooling/planet-gen/fix_fewshot_bleed.py +++ b/tooling/planet-gen/fix_fewshot_bleed.py @@ -152,47 +152,7 @@ def main(): print(f"WARNING: only {len(available)} replacements for {len(replacements_needed)} features") print(" some features will keep their few-shot names") - # Assign replacements deterministically — hash body_id + feature_id - # to pick from the pool, ensuring each body gets different names - used_per_body = defaultdict(set) - replacement_idx = 0 - changed_files = set() - total_replaced = 0 - - for markers_path, body_id, section, feat in replacements_needed: - old_name = feat["name"] - - # Find next available name not yet used on this body - assigned = None - for attempt in range(len(available)): - candidate = available[(replacement_idx + attempt) % len(available)] - if candidate.lower() not in used_per_body[body_id]: - assigned = candidate - replacement_idx = (replacement_idx + attempt + 1) % len(available) - break - - if assigned is None: - print(f" SKIP {body_id}/{section}: no unique replacement for \"{old_name}\"") - continue - - feat["name"] = assigned - used_per_body[body_id].add(assigned.lower()) - global_names.add(assigned.lower()) - changed_files.add(markers_path) - total_replaced += 1 - - # Write changed files - for markers_path in changed_files: - body_id = markers_path.parent.name - m = json.loads(markers_path.read_text()) - - # Re-apply changes (re-read since we modified feat objects in memory) - # Actually the feat dicts are still referenced — just rewrite - # But we need to reload and re-match since we didn't track which file - # has which changes... - - # Simpler approach: reload, replace, write - # Reset and do it properly + # Group by file, reload, replace, write replacement_idx = 0 used_per_body = defaultdict(set) changed_bodies = [] diff --git a/tooling/planet-gen/gemma_naming.py b/tooling/planet-gen/gemma_naming.py index 07a1e73cc..a8fd60cc6 100755 --- a/tooling/planet-gen/gemma_naming.py +++ b/tooling/planet-gen/gemma_naming.py @@ -71,14 +71,11 @@ DB_PATH = REPO_ROOT / "server" / "data" / "systems.db" WIKI_SYSTEMS = REPO_ROOT / "wiki" / "star-systems" BLOCKLIST_PATH = TOOLING_DIR / "earth_blocklist.txt" -# When --dump-prompts is set, name_feature short-circuits: it builds the -# attempt-0 prompt for each feature, writes a JSONL line to this file, -# and returns a unique deterministic placeholder so the pipeline runs to -# completion without touching a real LLM. Lets us capture the exact prompt -# set any backend (Gemma CPU, Gemma GPU, Haiku) would receive on first -# attempt — so an offline backend can replay them and the resulting names -# can be A/B compared. -_CAPTURE_FILE = None # set in main() when --dump-prompts is used +# NOTE: --dump-prompts and name_feature() are vestigial from the Gemma 2 +# single-name pipeline. The live path uses _batch_fill() → +# name_features_batch() from naming_core.py. The old code is retained +# for reference but not called. TODO(#833): remove in a cleanup pass. +_CAPTURE_FILE = None # vestigial — see note above # Default binary + model paths. The sr-voice binary is platform-specific # (GPU backend baked in per-build) and lives OUTSIDE any git worktree so @@ -562,7 +559,7 @@ _RIVER_POOLS: list[list[tuple[str, str]]] = [ ("Italian", "Fiume Marconi"), ("Greek", "Petrakis Rema"), ], - # Pool 6 — founder FIRST name possessive (Clifford's Bay shape) + # Pool 5 — founder FIRST name possessive (Clifford's Bay shape) # Added so first-name-possessive naming joins the rotation alongside # the surname pools without replacing any of them. [ @@ -573,7 +570,7 @@ _RIVER_POOLS: list[list[tuple[str, str]]] = [ ("Portuguese", "Rio de Ana"), ("French", "Rivière d'Elena"), ], - # Pool 5 — classical / institutional / Latinate (occasional ~17%) + # Pool 6 — classical / institutional / Latinate (occasional ~17%) [ ("British/Australian", "Aqueduct Run"), ("Italian", "Acqua Vetusta"), @@ -1973,7 +1970,7 @@ def discover_bodies( def main(): parser = argparse.ArgumentParser( - description="Batch-name atlas features via Gemma 2 voice pipeline (#833)" + description="Batch-name atlas features via Gemma 4 E2B tooling pipeline (#833)" ) parser.add_argument("--db", default=str(DB_PATH), help="Path to systems.db") parser.add_argument("--body", help="Process only this body_id") @@ -1995,7 +1992,7 @@ def main(): parser.add_argument( "--model", default=str(DEFAULT_MODEL), - help="Path to the Gemma 2 GGUF model (ignored in --mock mode)", + help="Path to the GGUF model file (ignored in --mock mode)", ) parser.add_argument( "--distrobox", @@ -2010,7 +2007,7 @@ def main(): "--refresh", type=int, default=1000, - help="Restart the voice subprocess every N requests (default: 200) " + help="Restart the voice subprocess every N requests (default: 1000) " "to prevent KV-cache context bleed", ) parser.add_argument( @@ -2120,7 +2117,7 @@ def main(): last_hop = hop_order.get(_body_id_from_path(markers_paths[-1])[0], (99, ""))[0] log.raw("") - log.raw(f" Gemma 2 Batch Naming Pipeline (#833)") + log.raw(f" Gemma 4 Batch Naming Pipeline (#833)") log.raw(f" DB: {db_path}") log.raw(f" Mode: {'MOCK' if args.mock else 'REAL'}") log.raw(f" sr-voice: {MOCK_STDIO if args.mock else sr_voice_bin}") diff --git a/tooling/planet-gen/prune_atlas_features.py b/tooling/planet-gen/prune_atlas_features.py index 13d9be318..c4135df0a 100755 --- a/tooling/planet-gen/prune_atlas_features.py +++ b/tooling/planet-gen/prune_atlas_features.py @@ -62,10 +62,11 @@ def prune_mountains(markers: dict, cap: int) -> int: mtns = markers.get("mountain_ranges") or [] if len(mtns) <= cap: return 0 + # Named features sort first (preserve hand-authored names), + # then by area_cells descending. ranked = sorted( mtns, - key=lambda m: int(m.get("area_cells") or 0), - reverse=True, + key=lambda m: (0 if m.get("name") else 1, -(int(m.get("area_cells") or 0))), ) markers["mountain_ranges"] = ranked[:cap] return len(mtns) - cap @@ -75,10 +76,11 @@ def prune_rivers(markers: dict, cap: int) -> int: rivers = markers.get("rivers") or [] if len(rivers) <= cap: return 0 + # Named features sort first (preserve hand-authored names), + # then by path length descending. ranked = sorted( rivers, - key=lambda r: len(r.get("path") or []), - reverse=True, + key=lambda r: (0 if r.get("name") else 1, -len(r.get("path") or [])), ) markers["rivers"] = ranked[:cap] return len(rivers) - cap