[codex] fix configurable mining and wing-aware dedup - #501
Conversation
web3guru888
left a comment
There was a problem hiding this comment.
This is a well-scoped fix for a real problem — wing-aware dedup addresses a footgun we've hit directly in multi-wing setups.
On the wing-aware dedup implementation:
The $and compound filter approach is correct for ChromaDB. One thing to watch: if a wing is None or empty string (e.g., a project without a configured wing falling back to default), the compound filter {"wing": wing} will filter differently than the old behavior. Would be worth asserting that wing is always a non-empty string at the call site before passing into file_already_mined.
On .al / .xlf / .ps1 additions to READABLE_EXTENSIONS:
PowerShell files (.ps1) are now both mineable and the hook implementation format — which means mining a project that includes the hooks themselves could create noise in the palace from the hook scaffolding. Not necessarily wrong (the hooks are legit project files), but worth being aware of.
On get_mining_config:
The extension normalization (ensuring leading dot, lowercasing) is solid. One edge case: if a user writes extensions: [""] (empty string in the YAML list), the if str(ext).strip() guard correctly skips it. Good defensive coding.
On the extra_skip_files fnmatch approach:
Using fnmatch against both relative_path and filename is right — patterns like *.g.al need the filename match. Did you consider patterns with path separators (e.g., generated/**)? fnmatch doesn't traverse / by default, so generated/** wouldn't match. Might be worth documenting the pattern scope in the config YAML schema or README.
The ✓ → + substitution: minor cosmetic, but removes a useful visual indicator on terminals that support Unicode. ✓ works fine in most modern terminals — is this purely for cp1252 compat, or is there a different reason?
Overall: the wing-aware dedup is the right default. --global-dedup as an explicit escape hatch is clean. Tests would make this merge-ready — particularly a test verifying that file_already_mined with wing_aware=True doesn't skip a file that was mined under a different wing.
web3guru888
left a comment
There was a problem hiding this comment.
This directly fixes the cross-wing dedup problem we ran into when mining our 5-domain setup. Previously, mining the same source files into separate wings (e.g., astrophysics and economics both reading shared methodology docs) meant the second wing got zero drawers. The wing-aware default + opt-out is the right call.
Wing-aware dedup — the query in is correct. One edge: if metadata is missing on old drawers (pre-wing-tagging), the filter will return 0 results and those files re-mine into every wing. That's actually the safe degradation path, but worth documenting.
**Configurable extensions via ** — clean design. The normalization (add leading if missing, deduplicate, lowercase) in handles the obvious user mistakes. One note: the config merge is additive (), so users can't remove default extensions, only add. That's probably fine for now but worth noting in docs.
** with ** — useful for generated-file patterns like . The dual-check ( against both and ) is important — without the relative path check, a pattern like wouldn't work. Good.
CLI output changes: replacing and with and for Windows cp1252 compat — valid. Same class of fix as #400.
No issues blocking merge. The wing-aware dedup alone makes this worth landing.
|
closing — dedup work landed via #544 (merged). thanks @vanachterjacob! |
Summary
mempalace.yamlwhile keeping.al,.xlf, and.ps1in the default readable setmining.extra_skip_filessupport for generated-file patterns such as*.g.al--global-dedupto restore cross-wing dedupWhy
Issue #278 identified two concrete problems during Windows and Business Central usage:
.alrepositories were mostly skipped because the miner could not ingest those files by default.source_file, so mining the same repo into multiple wings for different branches skipped later wings entirely.The maintainer asked for the broader fix shape in #278 (comment): configurable extensions plus wing-aware dedup with an opt-out for global behavior.
Validation
python -m mempalace.cli --helppython -m mempalace.cli mine --helppy_compileonmempalace/cli.py,mempalace/miner.py, andmempalace/convo_miner.pymempalace.yamlconfirming:.aland.rstfiles are ingested when configured*.g.alis skippedwing_aandwing_bfiles the same source files independently--global-dedupskips the same files when miningwing_cNotes
.rstsupport request in Allow .rst files #255.