Give Save Tag Changes a real channel-switch freshness check - #302
Merged
Osiris-DevWorks merged 57 commits intoJul 26, 2026
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A 2.2.0 user's Generate Enhancements crashed with UnicodeDecodeError:
byte 0xa0 — their cached base.ini wasn't clean UTF-8, on a stock
English install. 0xA0 is the Windows-1252 non-breaking space; CIG's
text carries plenty of characters that cp1252 encodes as single high
bytes (é / ™ / ° / em dash / NBSP), so a base.ini re-saved by an
external editor in ANSI (legacy Notepad's default), mangled by a
sync/AV tool, or a community-translated base saved as ANSI all decode
as invalid UTF-8 at the first such character.
Both readers opened the file with a strict utf-8-sig decode:
- scripts/generate_enhancements_ini.py's parse_ini crashed the whole
enhancements run (the reported traceback).
- src/parser/ini_parser.py's parse_ini_file was quietly worse: its
broad except caught the mid-iteration decode error and returned a
silently TRUNCATED result — every key after the bad byte vanished
with only a log warning.
Each now decodes the whole file up front — UTF-8 (BOM-aware) first,
falling back to cp1252 with errors="replace" (guards cp1252's five
undefined bytes) and a warning. Line splitting uses split('\n') +
rstrip('\r') rather than str.splitlines() so values containing
U+2028/U+0085 don't split where file iteration never did. The helper
is duplicated in both files deliberately: the generator script stays
stdlib+lxml-only (it also runs standalone), same shape as the existing
parse_ini/parse_ini_file parallelism.
Fixes Osiris-DevWorks#251
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lback Follow-up hardening pass on Osiris-DevWorks#251 after cross-checking the original Discord/Facebook report: a healthy install's extraction of the same LIVE data decodes as perfectly valid UTF-8, so the affected user's base.ini was machine-local corruption (possibly in their own Data.p4k, meaning re-extraction can reintroduce it) — one bad byte in an otherwise-valid file, not a wholesale ANSI re-save. That finding exposed a flaw in the initial fix: a blanket cp1252 fallback would mojibake every legitimate multi-byte character in a 10 MB UTF-8 file to repair one corrupt byte ("é" -> "é", thousands of times). The decode fallback now discriminates by measuring whether the UTF-8 replacement count tracks the file's high-byte count: corrupted UTF-8 gets errors="replace" (only the bad byte degrades to U+FFFD); a genuinely ANSI file still gets the cp1252 decode. The helper moved to src/utils/ini_io.read_ini_text so the merger can share it (merger already imports src.utils; parser->merger import direction forbids the reverse). The generator script keeps its own mirror to stay stdlib+lxml-only, documented as tolerated duplication. Audit of every other read of user-mutable/corruptible state found five more members of the same bug class, all fixed: - src/merger/ini_merger.py: merge_ini_files read base.ini with a strict open() — the SAME corrupt file also failed Apply to Game. Now routes through read_ini_text (newline shape preserved; existing merger tests plus two new ones pin it). - src/utils/user_ini_manager.py: _backup_if_changing runs on EVERY save and read_text raised UnicodeDecodeError past its OSError-only guard — one bad byte in user.ini bricked all saves. Now compares newline-normalized bytes (no decode at all); a corrupt file counts as "changing" and gets snapshotted before the clean rewrite. - src/utils/json_settings.py: a settings.json with corrupt bytes escaped the (OSError, JSONDecodeError) guard as UnicodeDecodeError and crashed the PORTABLE app at startup. Guard widened to ValueError (covers both). - src/utils/dataforge_diff.py: dirty_categories loaded the diff manifest unguarded — a corrupt manifest crashed the enhancements run. Now treated as "no manifest": regenerate everything and let update_manifest rewrite a clean one (self-healing). - src/utils/settings.py + scripts/generate_enhancements_ini.py stamp reads and src/utils/dataforge_patcher.py's locstring-workarounds loop: OSError-only (or JSONDecodeError-only) guards widened to also catch ValueError. Paths audited and already safe: blueprint_log_scanner (errors= "replace"), user_cfg (broad except; skipping is safer than rewriting a mangled decode into the game's own config), i18n/tutorial/docs loads (guarded with fallbacks), pak_extractor stamps (ValueError caught), applied_file_validator (broad except), should_autosave_user_ini (stat only, no decode). Registry-mode settings live in the Windows registry and have no file decode to corrupt. Fixes Osiris-DevWorks#251 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…g as @keyname Same fix as issue/261-global-ini-missing-bom (release/2.2.1), reapplied here since this branch predates the Osiris-DevWorks#251 encoding rewrite (that's PR Osiris-DevWorks#253, separately targeting this same branch) and still has the older merge_ini_files shape (plain open() calls, not read_ini_text). Critical bug, confirmed on a tester's real Star Citizen install (not yet published to end users): after Apply, the entire in-game UI — main menu, mobiGlas, contacts, everything — showed raw loc keys instead of resolved text. Deleting Smart Citizen's global.ini restored correct text immediately. Root cause: Data.p4k's own extracted global.ini ships with a UTF-8 BOM (confirmed directly — raw unp4k output starts with \xef\xbb\xbf). Star Citizen's own localization loader appears to need that BOM to reliably detect the file's encoding; without it, the engine can fail to resolve the ENTIRE loc table rather than degrading per-key. merge_ini_files has written plain UTF-8 with no BOM in every prior release. Two-part fix: - merge_ini_files now opens both the source (utf-8-sig, so a source BOM doesn't leak into the first key's name either — a related bug this branch's older code shape also had) and the output (utf-8-sig, so the written file gets the BOM) with BOM-aware encoding. - validate_applied_file now treats a missing BOM as a first-class validation failure (not just key presence), triggering the existing rollback-to-backup path. Fixes Osiris-DevWorks#261 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…me corruption Ported from issue/257-starmap-name-corruption (originally targeted release/2.2.1, now retired in favor of consolidating everything onto release/2.3.0). Same fix, same tests — see Osiris-DevWorks#255/Osiris-DevWorks#257 for the full root-cause writeup. Critical bug: applying Smart Citizen's output renamed the Crusader planet to "Stanton (Star)" on the in-game starmap. Root cause: sync_key_variants() canonicalized every key in the ~90k-key table (strip _SCItem, lowercase, strip all underscores) to reconcile CIG's inconsistent ship-component naming, but two completely unrelated keys — Stanton2 (the planet) and Stanton_2 (the star) — collapsed to the same canonical string by coincidence, and the longest-value tie-break overwrote the planet's name. A real base.ini audit found 14 total live corruptions of this shape (comm-array button labels, a mission title, Options-menu turret labels, two distinct thrusters collapsed to one name, a hardpoint slot losing its distinguishing suffix, and more) — not an isolated incident. Fix: scope the canonical-key grouping to item_name/item_desc-prefixed keys only — the exact domain this sync was ever designed and tested for. Fixes Osiris-DevWorks#255 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ow show XenoThreat's "Purgatory Camo" armor/weapon/component set was announced purely on CIG's website for a limited-time event, never advertised via any mission's POTENTIAL BLUEPRINTS text -- so mission-text scanning alone could never surface it, leaving these items visible only in the Owned list (via the log scanner's independent "Received Blueprint: ..." matching) and never in Available. Adds a manually-curated MANUAL_BLUEPRINT_ITEMS list folded into build_blueprint_metadata()'s output: real tag/class/size/grade if a matching item_Name entry is actually loaded, a bare fallback entry otherwise, never overwriting a real mission-derived entry. Tooltip reads "Limited time event reward" instead of a mission name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…local drives The only auto-detection fallback checked two hardcoded C:\Program Files\... paths, so a fresh profile (or a portable build's first run, no saved settings yet) showed the "Star Citizen Path Required" dialog even when a real install existed -- a tester's real install lives at E:\Games\Roberts Space Industries\StarCitizen, a shape neither hardcoded candidate could ever match. _scan_common_sc_install_locations() now checks every local drive letter against RSI Launcher's own default shape, both Program Files bitness variants, a bare secondary-drive install, and one nested under a personal "Games" folder -- validated the same way every other candidate already is. Wired into both get_sc_install_root() and get_game_install_path(), persisting the result once found so the scan only ever runs once per profile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…Osiris-DevWorks#273) _enhancements_dirty (the sole gate on the Generate Enhancements button's enabled state) was only ever computed once at tab construction, then nudged True/False by discrete UI events afterward (a checkbox toggle, clicking Generate itself) -- never recomputed wholesale. Switching channels changes what "needs regeneration" even means, but the flag was left untouched: if the previous channel ended clean, the button stayed disabled for a new channel that actually did need (re)generation, silently swallowing clicks until the user happened to retoggle some checkbox that forces it back on (the "untick and retick" workaround reported in the issue). Adds EnhancementsTab.refresh_enhancements_dirty_state(), a public entrypoint (same shape as the existing mark_enhancements_dirty()) that re-derives the flag from the new channel's real disk state, and calls it from MainWindow._on_channel_changed right alongside the existing refresh_enhancements_status() call.
…siris-DevWorks#259) Toggling "Underline Direct" (and 6 other checkboxes on this page) updated self.config and refreshed the preview but never emitted config_changed, so MainWindow's _mark_tag_dirty() never fired and the Save Tag Changes button stayed disabled despite a real pending change. Every other control on this page (the 5 dropdowns, General Tags/ Scanning checkboxes) already emitted the signal correctly. Found a 4th affected handler while fixing the reported 3: _on_mt_standardize_toggle (the "Standardize hauling mission names" checkbox) had the exact same bug — 7 checkboxes total, not 6. Redo of the original fix from PR Osiris-DevWorks#260, which targeted the abandoned release/2.2.1 branch and was closed unmerged; recreated here against release/2.3.0 after confirming the bug and the widget attribute names are unchanged.
…/266-blueprint-general-fixes # Conflicts: # VERSION.TXT
…annotations Two compounding bugs found while wiring up Scraper Module tagging, both affecting far more than that one item: - _bp_section_span mistook a reputation-tiered contract's "Awarded from X level variants" sub-header for a genuine section boundary (it doesn't start with "[" like a region label does), truncating the scan before any bullets were ever reached. Confirmed via the fixture data that this pattern spans dozens of missions (Adagio Industrial, Bounty Hunters Guild, Security, ...), not just salvage contracts. - normalize_item_name never stripped the "(Category)" annotation CIG appends to some bullets (Bendix (Fuel Nozzle), Arbor MH1 Mining Laser (Mining Laser), 5CA 'Akura' (Shield), Trawler Scraper Module (Salvage Mod)), so even a correctly-scanned bullet never matched its real item_Name entry. Both silently kept affected items out of the Blueprint Tracker entirely (not just untagged -- absent), regardless of any Tag Builder configuration.
…rks#266 follow-up) CIG uses a second, entirely different section header -- "MULTIPLE BLUEPRINT POOLS" -- for missions that offer more than one blueprint pool (e.g. mining-laser purchase-order contracts awarding a weapon/armor Pool 1 alongside a mining-laser/radar Pool 2). Confirmed via tests/fixtures/kraken_global_latest.ini: 35 missions use it against 237 using "POTENTIAL BLUEPRINTS". Only the latter was recognised at all, so these missions' blueprint bullets were never scanned -- not just untagged, entirely absent from the Blueprint Tracker and from the String Editor's "BP Descriptions" filter. Adds has_bp_section() as the single header-detection entry point (replacing three separate raw substring checks in blueprint_meta.py and entry_filter.py) and recognises "Pool 1"/"Pool 2"/... sub-labels inside the section as non-boundary markers, the same way region labels and reputation-tier labels already were.
…Osiris-DevWorks#266 follow-up) Live mission body ("Crew Hasn't Checked In") reported bullets that match no real item_Name value at all, keeping those items out of the Blueprint Tracker entirely -- two distinct root causes: - CIG's own bug: some bullets list the de-slugified loc KEY instead of the item's localized name -- "Nozzle Fuelgiver Grin Nozzleveryfast" for a key whose real value is "Lindstrom" (Nozzle_FuelGiver_GRIN_NozzleVeryFast_Name). Deterministic (title-case each underscore segment, dropping a trailing "_Name" first), so resolved generically via _key_slug rather than one alias per affected item. - A one-off mismatch with no deterministic pattern: "Helix" bears no relation to its key's slug, just an informal short name a mission author typed; the real item is "S0 Helix" (item_NameMining_Head_S00_Helix_SCItem). Added as a small explicit alias, same precedent as Osiris-DevWorks#267's MANUAL_BLUEPRINT_ITEMS. Both only apply as a fallback when the bullet name doesn't already match a real item directly.
…s-DevWorks#266 follow-up) Hofstede reported with the same "Helix" mismatch (mission bullet uses the bare manufacturer/model name, real item is "S00 Hofstede" via item_NameMining_Head_S00_Hofstede_SCItem) -- confirms this is a whole-family pattern, not a one-off. Adds Arbor and Klein preemptively too, same key convention (item_NameMining_Head_S00_<Name>_SCItem), rather than waiting for each to be reported individually. Klein's real name ("Lawson Mining Laser") breaks the "S0 <Name>" pattern entirely, so it couldn't have been caught by any general rule either way.
…vWorks#266 follow-up) Confirmed via a live Blueprint Tracker screenshot: 5 of 8 fuel nozzle variants resolved correctly through the generic _key_slug reversal, but Norfield, Harkin, and RN-7s still showed up as separate untagged "Nozzle Fuelgiver ..." entries -- their real underlying keys apparently don't follow the Nozzle_FuelGiver_<MFR>_Nozzle<Variant>_Name pattern the other five do. Added as explicit aliases, same as the Mining Head family.
…DevWorks#266 general fix) Nozzles ship under item_fuelnozzle_*/Nozzle_FuelGiver_* prefixes, not item_Name*/vehicle_Name*, so Pass 1 never saw these keys as Name entries at all -- the alias table and _key_slug fallback added by the prior two commits had nothing to resolve against, and every nozzle still showed up as a separate untagged/garbled bullet. Scoped to the two fuel-nozzle prefixes only; the ship-mounted mining-laser and scraper-module prefixes are new-tag-feature territory for 2.3.0, not a 2.2.1 bug fix.
…Works#251) The JSONDecodeError->ValueError exception widening in load_locstring_workarounds was the one Osiris-DevWorks#251-class change shipped without a test exercising the new branch -- its three sibling files (json_settings, dataforge_diff, user_ini_manager) all got the identical change with matching tests in TestCorruptStateFileGuards. Found by the pre-release test-coverage audit.
Testers on the 2.2.1 preview build need plan items covering Osiris-DevWorks#251, Osiris-DevWorks#261, Osiris-DevWorks#257, Osiris-DevWorks#267, Osiris-DevWorks#269, Osiris-DevWorks#273, Osiris-DevWorks#259, Osiris-DevWorks#247, and the general Blueprint Tracker parsing fixes ported from Osiris-DevWorks#266 -- none of it was represented in TEST_SECTIONS yet.
… and Apply Owned Tags Reported while live-testing the first Osiris-DevWorks#273 fix: the same stale-dirty-state class hit two more buttons on a channel switch. - Save Tag Changes: tag configs are global but the generated INIs are per-channel, and nothing records which configs a channel's INIs were last generated with. After a switch the button kept the previous channel's clean state, leaving the user unable to stamp their tags onto the new channel at all. Now lit on switch (erring on clickable — worst case is one redundant regeneration). - Apply Owned Tags: only the button's own click ever cleared its flag, so after a switch it sat grey ("no changes") and the user couldn't force a re-weave for the new channel. Now marked via the existing public mark_owned_dirty() on switch; the flag survives the subsequent reload since nothing in that path clears it.
Save Tag Changes and Apply Owned Tags now also refresh on a channel switch; testers need an item exercising all three buttons, not just Generate Enhancements.
Crash report: opening the Custom... resolution in Import INI and entering
a value raised TypeError: tr() got multiple values for argument 'key'.
import_dialog.py's custom_value_prompt call already existed on this
branch as tr("import_dialog.custom_value_prompt", key=key) -- passing a
loc key as an interpolation variable is a very natural thing to do, but
tr(key: str, **kwargs) has its own first parameter also named "key", so
Python rejects the call before the function body ever runs.
Renames tr()'s own parameter to i18n_key so "key" is free for any caller,
and fixes the one existing crashing call site (custom_value_prompt) plus
its {key} -> {loc_key} placeholder across all four locale files.
This is scoped to the one pre-existing crash. A second, unrelated
tr()-with-key= collision in main_window.py's "Copy Key" action only
exists once Osiris-DevWorks#247's hardcoded-string sweep converts that line from a
plain f-string to a tr() call -- that one is fixed alongside the sweep
itself in release/2.3.0, not here.
…siris-DevWorks#281) CIG bug: all 8 fuel nozzle blueprint rewards miss both the UUID and filename entity-name resolution tiers in build_blueprint_pool_lookup, falling through to _name_from_blueprint_filename's title-cased-slug fallback. Confirmed live in-game via an "URGENT REFUEL REQUEST: Drake Cutter" mission body listing all 8 variants as raw garbled text (e.g. "Nozzle Fuelgiver Grin Nozzlefast") instead of their real manufacturer names in the generated global.ini that Apply to Game writes. Adds a known-alias correction keyed by the exact fallback string this function produces, scoped to the 8 confirmed fuel nozzle variants -- does not touch resolution for any other blueprint type. This is a generation-time fix (changes what's written to global.ini), distinct from blueprint_meta.py's alias table which only affects Smart Citizen's own Blueprint Tracker tab display and never touches in-game text.
…siris-DevWorks#220) Community overview video of Smart Citizen's features, credited and linked in both README.md and the in-app About panel.
Leftover from before this branch was retargeted from release/2.3.0 to release/2.2.1 — the version-bump commit never got updated, so the PR diff wrongly showed VERSION.TXT changing to 2.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Leftover from before this branch was retargeted from release/2.3.0 to release/2.2.1 — the version-bump commit never got updated, so the PR diff wrongly showed VERSION.TXT changing to 2.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Leftover from before this branch was retargeted from release/2.3.0 to release/2.2.1 — the version-bump commit never got updated, so the PR diff wrongly showed VERSION.TXT changing to 2.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Leftover from before this branch was retargeted from release/2.3.0 to release/2.2.1 — the version-bump commit never got updated, so the PR diff wrongly showed VERSION.TXT changing to 2.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Leftover from before this branch was retargeted from release/2.3.0 to release/2.2.1 — the version-bump commit never got updated, so the PR diff wrongly showed VERSION.TXT changing to 2.3.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g fixes # Conflicts: # VERSION.TXT
…g fixes # Conflicts: # VERSION.TXT
…, stale Osiris-DevWorks#247 item The 2.2.1 bug-fix bundle section title cited Osiris-DevWorks#257 (closed as a duplicate of Osiris-DevWorks#255 -- the actual open tracking issue for the Crusader starmap corruption) and Osiris-DevWorks#247 (the i18n hardcoded-string sweep, which lives on release/2.3.0 via PR Osiris-DevWorks#252 and was never merged into release/2.2.1). Testers checking the Test Plan against this release would have been looking for a language-switch fix that isn't actually in the build. Also: Osiris-DevWorks#281 (garbled fuel nozzle names) was completely missing from the title despite being one of the 8 issues labeled 2.2.1, and had no dedicated test item -- the existing fuel-nozzle item only covers the Blueprint Tracker tab's own display (Osiris-DevWorks#278's fix), not the in-game mission text Osiris-DevWorks#281 actually fixes (per PR Osiris-DevWorks#283's own description, these are explicitly two different bug classes). Added a matching test item for the in-game case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…siris-DevWorks#255 Osiris-DevWorks#257 was closed as a duplicate of Osiris-DevWorks#255 (the canonical, open tracking issue for the Crusader starmap corruption), but the actual fix code and its test class still cited Osiris-DevWorks#257 throughout -- in ini_merger.py's docstrings/comments and in test_ini_merger.py's module docstring, class docstring, and PR-referencing comment. Anyone reading this code later and looking up Osiris-DevWorks#257 would land on a closed duplicate with no context, instead of Osiris-DevWorks#255's full root-cause writeup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…isible BOM chars in tests Two suggestions from review, both fixed: - validate_applied_file returned "" (validation passed) whenever the key-presence comparison couldn't run at all -- base.ini missing from cache, unreadable, or the written file itself failing to parse -- without ever looking at the has_bom result it had already computed. A BOM-less written file on a profile with no cached base.ini would silently pass validation instead of triggering rollback. The BOM check now stands on its own and its warning survives every early return. - Two tests in test_ini_merger.py typed the BOM as a literal U+FEFF character embedded in a quoted string, invisible in an editor and easy to lose on copy/paste. Switched to explicit b"\xef\xbb\xbf" bytes on the write side and chr(0xFEFF) on the assertion side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ore persisting Two suggestions from review, both fixed: - _scan_common_sc_install_locations walked all 26 drive letters on every call, including a "found nothing" outcome that never got remembered. A disconnected network drive can make a single exists() check hang for seconds, so a no-install profile paid that cost on every call (e.g. every channel switch clears GAME_INSTALL_PATH and triggers a fresh lookup). Now cached in-memory for the process's lifetime (resets naturally on restart, not persisted to settings). - get_game_install_path() saved root/channel from the drive scan without checking the channel folder actually exists -- the scan only proves SOME channel folder exists at that root (via _is_valid_sc_root), not necessarily the active one. Now only persists when Path(result).is_dir() is true; the guessed path is still returned either way, but an unconfirmed guess no longer gets cached for a later read to trust and then discard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arg-crash-fix Fix crash: tr() kwarg named 'key' collides with its own first parameter
…el-switch-dataforge-detection [Osiris-DevWorks#273] Fix Generate Enhancements button stuck disabled after channel switch
…on-titles-dirty-state [Osiris-DevWorks#259] Fix Mission Titles page: 7 checkboxes never armed Save Tag Changes
…-global-ini-missing-bom-2.3.0 [Osiris-DevWorks#261] Write applied global.ini with a UTF-8 BOM — fixes @keyname corruption
…detect-sc-install-path [Osiris-DevWorks#269] fix: auto-detect Star Citizen install across all local drives
…nozzle-blueprint-name-fallback [Osiris-DevWorks#281] Fix garbled fuel nozzle names in mission POTENTIAL BLUEPRINTS lists
…rint-general-fixes [Osiris-DevWorks#266] Fix general Blueprint Tracker bullet-matching bugs
# Conflicts: # tests/test_ini_merger.py
…-starmap-name-corruption-2.3.0 [Osiris-DevWorks#255] Fix starmap/UI name corruption: scope sync_key_variants() to item keys
…eadiness 2.2.1 release readiness: test coverage gap + Test Plan section
Brings in the full 2.2.1 bug-fix bundle (Osiris-DevWorks#251, Osiris-DevWorks#255, Osiris-DevWorks#259, Osiris-DevWorks#261, Osiris-DevWorks#267, Osiris-DevWorks#269, Osiris-DevWorks#273, Osiris-DevWorks#281) so this 2.3.0 branch doesn't repeat the missing-2.2.1-fixes problem release/2.3.0 itself had before Osiris-DevWorks#289.
…ess checks On channel switch, Generate Enhancements already does a real freshness check (refresh_enhancements_dirty_state) and only lights when the new channel genuinely needs regeneration. The two sibling buttons instead lit unconditionally on every switch. This aligns them with Generate Enhancements so each only lights when there's real pending work. Apply Owned Tag (blueprint tracker): - The [Owned] weave (_recompute_owned) runs synchronously on every reload and owned-set change, so there is never pending owned-tag work right after a channel switch — the weave already matches the (global) owned set for whatever the new channel loaded. The async-extraction path is covered too: the deferred reload runs _recompute_owned when the new entries first exist. So on switch we now mark_owned_clean() instead of mark_owned_dirty(). Same class as Osiris-DevWorks#296 (button lit red for work already done); the button now only lights on real owned-set edits. Save Tag Changes (enhancements tab): - Unlike owned tags, a channel's generated *_enhancements.ini CAN be genuinely stale vs the current Tag Builder config, so this gets a real check rather than just going clean. Added a per-channel/per-language ".tag_config_stamp" that mirrors the existing ".dataforge_stamp": - tag_builder.tag_config_fingerprint() — deterministic sha256[:12] of the tag configs (TagConfig.to_json is already canonical) plus the annotate-mission-descriptions toggle. Covers exactly what the button tracks; excludes scope/target inputs (enabled categories, language). - The generator worker writes the stamp beside set_enhancements_stamp, from the configs it actually generated with (self.*), so the stamp reflects the generated output. - refresh_tag_builder_dirty_state() now lights the button only when the channel's stamp is missing or differs from a fingerprint of the LIVE Tag Builder pages. Using live page state (not persisted settings) keeps unsaved in-session edits correctly lit across a switch; a missing stamp reads dirty, so pre-existing channels behave exactly as before until regenerated. - AppSettings.get/set_tag_config_stamp + get_current_tag_config_finger print mirror the enhancements-stamp helpers, incl. the Osiris-DevWorks#251 corrupt- stamp (OSError, ValueError) robustness. Tests: 7 fingerprint tests (determinism, order-independence, edit- and annotate-sensitivity, frozenset-order equivalence) and 6 stamp tests (per-language isolation, independence from the DataForge stamp, corrupt stamp -> "", stamp file location).
The prior commit made Apply Owned Tags mark_owned_clean() on channel switch, reasoning that _recompute_owned() already re-weaves on every reload so there's no genuine per-channel staleness left to prompt for. That reasoning holds for whether the *current* weave is accurate, but it took away something the user actually wants: staying clickable after a switch so they can deliberately force-reapply owned tags to the newly-selected channel, the same as before this change (matches the button's original Osiris-DevWorks#273 behavior — restored verbatim, including the original call and comment). Save Tag Changes' real freshness check (the .tag_config_stamp) is unaffected and confirmed working.
Stealrull
added
Bug
Collaborator
|
@claude review |
Osiris-DevWorks
previously approved these changes
Jul 26, 2026
Osiris-DevWorks
left a comment
Osiris-DevWorks
left a comment
Owner
There was a problem hiding this comment.
Approving. The freshness-check feature is correct, well-tested, and standards-clean.
- Per-channel/per-language
.tag_config_stampmirrors.dataforge_stamp; the stamp lives underget_enhancements_dir(), so a switch reads the target channel's stamp — it genuinely detects the switch. - Live-page fingerprint keeps unsaved edits lit across a switch; a missing stamp reads dirty, so un-stamped channels behave exactly as before.
- Worker stamp, live fingerprint, and
get_current_tag_config_fingerprint()iterate the sameCATEGORIESset — no spurious mismatch. Fingerprint inputs are pre-fetched on the main thread and handed to the worker (honors the tag-config hand-off rule); corrupt-stamp guard matches the existing pattern. - Deliberate Apply-Owned-Tag exclusion is documented and correct.
The bundled 2.2.1 noise that inflated this diff is resolved now that #289 has merged.
Generated by Claude Code
Stealrull
dismissed
Osiris-DevWorks’s stale review
July 26, 2026 19:30
The merge-base changed after approval.
Osiris-DevWorks
merged commit Jul 26, 2026
f1c00f0
into
Osiris-DevWorks:release/2.3.0
6 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On channel switch, Generate Enhancements already does a real freshness check (
refresh_enhancements_dirty_state) and only lights up when the new channel genuinely needs regeneration. Save Tag Changes instead lit up unconditionally on every switch, since nothing recorded which Tag Builder config a channel's generated INIs were built from.Fix
Added a per-channel/per-language
.tag_config_stampfile, mirroring the existing.dataforge_stamp:tag_builder.tag_config_fingerprint()— deterministic sha256[:12] of the tag configs (TagConfig.to_json()is already canonical) plus the annotate-mission-descriptions toggle.set_enhancements_stamp, from the configs it actually generated with.refresh_tag_builder_dirty_state()now lights the button only when the channel's stamp is missing or differs from a fingerprint of the live Tag Builder pages (not just persisted settings) — so unsaved in-session edits still correctly keep the button lit across a switch. A missing stamp reads dirty, so pre-existing channels behave exactly as before until regenerated.I also looked at giving Apply Owned Tag (Blueprint Tracker) the same treatment, since its
[Owned]weave always re-runs on reload and technically has no per-channel staleness to detect. I implemented that too, but reverted it — the button staying clickable after a switch is a deliberate, wanted affordance (force-reapply owned tags to the newly-selected channel), not a bug, so that behavior is unchanged from currentrelease/2.3.0.Testing
Adversarially reviewed (4 independent review passes + skeptic verification on each finding) before landing — confirmed no false-clean/false-dirty holes, including the async-extraction deferred-reload path. Added 13 new/updated tests (fingerprint determinism/order-independence/sensitivity, stamp round-trip/isolation/corrupt-stamp handling). Built a portable and confirmed in the real app: Save Tag Changes stays grey across a channel switch when nothing changed, lights up when the config genuinely differs, and unsaved edits survive a switch.