diff --git a/.changelog/029.yaml b/.changelog/029.yaml new file mode 100644 index 0000000..b341126 --- /dev/null +++ b/.changelog/029.yaml @@ -0,0 +1,28 @@ +name: main +ts: 2026-04-12 16:10:19.714611+00:00 +type: keep_private +full_path: _internal.cli.main +--- +name: workflows +ts: 2026-04-12 16:10:30.816480+00:00 +type: fix +author: Espen Albert +changelog_message: 'fix: Resolve short-name collision that silently drops duplicate symbol names' +message: 'fix: Resolve short-name collision that silently drops duplicate symbol names' +short_sha: 8d9632 +--- +name: workflows +ts: 2026-04-12 16:10:51.908835+00:00 +type: fix +author: Espen Albert +changelog_message: 'fix: Update decided_local_ids for moved symbols during reconciliation' +message: 'fix: Update decided_local_ids for moved symbols during reconciliation' +short_sha: 5636ab +--- +name: workflows +ts: 2026-04-12 16:11:02.339052+00:00 +type: fix +author: Espen Albert +changelog_message: 'fix: Reconcile owned_modules alongside owned_refs when symbols move' +message: 'fix: Reconcile owned_modules alongside owned_refs when symbols move' +short_sha: dddfde diff --git a/.groups.yaml b/.groups.yaml index 4c3645d..86b55a7 100644 --- a/.groups.yaml +++ b/.groups.yaml @@ -47,7 +47,6 @@ groups: - _internal.cli.gen_cmds owned_refs: - _internal.cli.gen_cmds.gen_docs - - _internal.cli.gen_cmds.gen_tests - name: stability owned_modules: - _internal.cli.stability_cmds diff --git a/pkg_ext/_internal/changelog/write_changelog_md.py b/pkg_ext/_internal/changelog/write_changelog_md.py index 7816f7c..33a9f40 100644 --- a/pkg_ext/_internal/changelog/write_changelog_md.py +++ b/pkg_ext/_internal/changelog/write_changelog_md.py @@ -102,8 +102,8 @@ def as_changelog_line(action: ChangelogAction, remote_url: str, ctx: pkg_ctx) -> return "" case FixAction(message=msg, changelog_message=cl_msg, short_sha=sha): return f"{cl_msg or msg} {_commit_url(remote_url, sha)}" - case MakePublicAction(name=name): - return f"New {ctx.code_state.ref_symbol(name).type} `{name}`" + case MakePublicAction(name=name, full_path=full_path): + return f"New {ctx.code_state.ref_symbol(full_path or name).type} `{name}`" case DeleteAction(name=name, group=group): return f"Removed `{group}.{name}`" case RenameAction(name=name, old_name=old_name, group=group): diff --git a/pkg_ext/_internal/cli/workflows.py b/pkg_ext/_internal/cli/workflows.py index e92daf6..7a2a0b0 100644 --- a/pkg_ext/_internal/cli/workflows.py +++ b/pkg_ext/_internal/cli/workflows.py @@ -104,7 +104,7 @@ def create_ctx(api_input: GenerateApiInput) -> pkg_ctx: with exit_stack: code_state = parse_pkg_code_state(settings) tool_state, extra_actions = parse_changelog(settings, code_state) - tool_state.groups.reconcile_moved_refs(code_state.import_id_refs) + tool_state.reconcile_with_code(code_state.import_id_refs) git_changes_input = GitChangesInput( repo_path=settings.repo_root, @@ -223,7 +223,7 @@ def clean_old_entries(settings: PkgSettings): def create_stability_ctx(settings: PkgSettings) -> pkg_ctx: code_state = parse_pkg_code_state(settings) tool_state, extra_actions = parse_changelog(settings, code_state) - tool_state.groups.reconcile_moved_refs(code_state.import_id_refs) + tool_state.reconcile_with_code(code_state.import_id_refs) return pkg_ctx( settings=settings, diff --git a/pkg_ext/_internal/models/code_state.py b/pkg_ext/_internal/models/code_state.py index 680666b..35ff08a 100644 --- a/pkg_ext/_internal/models/code_state.py +++ b/pkg_ext/_internal/models/code_state.py @@ -41,14 +41,25 @@ def validate_and_prepare(self): self.files.sort() return self - def ref_symbol(self, name: str) -> RefSymbol: - if ref_state := self.named_refs.get(name): - return ref_state.symbol - raise RefSymbolNotInCodeError(name) + def ref_symbol(self, name_or_local_id: str) -> RefSymbol: + if ref := self.import_id_refs.get(name_or_local_id): + return ref + matches: list[RefSymbol] = [] + for ref in self.import_id_refs.values(): + if ref.local_id == name_or_local_id: + return ref + if ref.name == name_or_local_id: + matches.append(ref) + if len(matches) == 1: + return matches[0] + if matches: + local_ids = [m.local_id for m in matches] + raise RefSymbolNotInCodeError(f"{name_or_local_id} is ambiguous, matches: {local_ids}") + raise RefSymbolNotInCodeError(name_or_local_id) @property def named_refs(self) -> dict[str, RefStateWithSymbol]: - return {ref.name: RefStateWithSymbol(name=ref.name, symbol=ref) for ref in self.import_id_refs.values()} + return {ref.local_id: RefStateWithSymbol(name=ref.name, symbol=ref) for ref in self.import_id_refs.values()} def lookup(self, ref: RefSymbol) -> Any: full_reference_id = ref.full_id(self.pkg_import_name) diff --git a/pkg_ext/_internal/models/groups.py b/pkg_ext/_internal/models/groups.py index ee8dfe9..ff0edc7 100644 --- a/pkg_ext/_internal/models/groups.py +++ b/pkg_ext/_internal/models/groups.py @@ -12,7 +12,7 @@ from pkg_ext._internal.errors import InvalidGroupSelectionError, NoPublicGroupMatch from .py_symbols import RefSymbol -from .types import PyIdentifier, SymbolRefId, ref_id_name +from .types import PyIdentifier, SymbolRefId, ref_id_module, ref_id_name if TYPE_CHECKING: # why type checking? Can we not use the root import? from pkg_ext._internal.config import ProjectConfig @@ -145,13 +145,17 @@ def merge_config(self, config: ProjectConfig) -> None: group.docs_exclude = group_cfg.docs_exclude.copy() group.docstring = group_cfg.docstring - def reconcile_moved_refs(self, import_id_refs: dict[str, RefSymbol]) -> int: - """Auto-fix refs that have moved to different modules.""" + def reconcile_moved_refs(self, import_id_refs: dict[str, RefSymbol]) -> tuple[int, set[SymbolRefId]]: + """Auto-fix refs and modules that have moved to different paths. + + Returns (count_of_updates, set_of_moved_to_ref_ids). + """ name_to_candidates: dict[str, list[RefSymbol]] = {} for ref in import_id_refs.values(): name_to_candidates.setdefault(ref.name, []).append(ref) total_updated = 0 + moved_to: set[SymbolRefId] = set() for group in self.groups: updated_refs: set[SymbolRefId] = set() for ref_id in list(group.owned_refs): @@ -165,21 +169,29 @@ def reconcile_moved_refs(self, import_id_refs: dict[str, RefSymbol]) -> int: if current_id != ref_id: logger.warning(f"Symbol moved: {ref_id} -> {current_id} (group: {group.name})") updated_refs.add(current_id) + moved_to.add(current_id) total_updated += 1 else: updated_refs.add(ref_id) elif resolved := self._resolve_ambiguous_ref(group, ref_id, candidates): if resolved != ref_id: logger.warning(f"Symbol moved: {ref_id} -> {resolved} (group: {group.name})") + moved_to.add(resolved) total_updated += 1 updated_refs.add(resolved) else: updated_refs.add(ref_id) # unresolved - keep stale ref group.owned_refs = updated_refs + self._reconcile_owned_modules(group, updated_refs) if total_updated: self.write() - return total_updated + return total_updated, moved_to + + @staticmethod + def _reconcile_owned_modules(group: PublicGroup, updated_refs: set[SymbolRefId]) -> None: + """Rebuild owned_modules to match the modules referenced by updated_refs.""" + group.owned_modules = {ref_id_module(ref) for ref in updated_refs} def _resolve_ambiguous_ref( self, group: PublicGroup, stale_ref_id: SymbolRefId, candidates: list[RefSymbol] diff --git a/pkg_ext/_internal/models_test.py b/pkg_ext/_internal/models_test.py index 1b929e5..6f762f0 100644 --- a/pkg_ext/_internal/models_test.py +++ b/pkg_ext/_internal/models_test.py @@ -1,4 +1,5 @@ from datetime import UTC, datetime +from pathlib import Path from typing import Callable import pytest @@ -6,16 +7,22 @@ from pkg_ext._internal.changelog.actions import ( GroupModuleAction, + KeepPrivateAction, MakePublicAction, changelog_filepath, dump_changelog_actions, ) from pkg_ext._internal.changelog.parser import parse_changelog +from pkg_ext._internal.errors import RefSymbolNotInCodeError from pkg_ext._internal.models import ( + PkgCodeState, PublicGroups, RefSymbol, SymbolType, ) +from pkg_ext._internal.models.py_files import PkgSrcFile +from pkg_ext._internal.models.ref_state import RefState, RefStateType +from pkg_ext._internal.pkg_state import PkgExtState from pkg_ext._internal.settings import PkgSettings @@ -86,7 +93,9 @@ def test_reconcile_moved_refs_no_changes(_public_groups): _public_groups.get_or_create_group("my_group").owned_refs.add("_internal.models.MyClass") refs = _refs_dict(_ref("MyClass", "_internal/models")) - assert _public_groups.reconcile_moved_refs(refs) == 0 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 0 + assert not moved_to assert "_internal.models.MyClass" in _public_groups.name_to_group["my_group"].owned_refs @@ -96,9 +105,13 @@ def test_reconcile_moved_refs_updates_moved_symbol(_public_groups, caplog): group.owned_modules.add("_internal.old_module") refs = _refs_dict(_ref("MyClass", "_internal/new_module")) - assert _public_groups.reconcile_moved_refs(refs) == 1 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 1 + assert "_internal.new_module.MyClass" in moved_to assert "_internal.new_module.MyClass" in group.owned_refs assert "_internal.old_module.MyClass" not in group.owned_refs + assert "_internal.new_module" in group.owned_modules + assert "_internal.old_module" not in group.owned_modules assert "Symbol moved:" in caplog.text @@ -106,7 +119,9 @@ def test_reconcile_moved_refs_keeps_deleted_symbol(_public_groups): group = _public_groups.get_or_create_group("my_group") group.owned_refs.add("_internal.models.DeletedClass") - assert _public_groups.reconcile_moved_refs({}) == 0 + count, moved_to = _public_groups.reconcile_moved_refs({}) + assert count == 0 + assert not moved_to assert "_internal.models.DeletedClass" in group.owned_refs @@ -117,7 +132,9 @@ def test_reconcile_moved_refs_multiple_groups(_public_groups, caplog): group2.owned_refs.add("_internal.old.Func2") refs = _refs_dict(_ref("Func1", "_internal/new"), _ref("Func2", "_internal/new")) - assert _public_groups.reconcile_moved_refs(refs) == 2 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 2 + assert moved_to == {"_internal.new.Func1", "_internal.new.Func2"} assert "_internal.new.Func1" in group1.owned_refs assert "_internal.new.Func2" in group2.owned_refs @@ -128,7 +145,9 @@ def test_reconcile_disambiguates_via_owned_modules(_public_groups): group.owned_modules.add("_internal.models") refs = _refs_dict(_ref("Parser", "_internal/models"), _ref("Parser", "_internal/utils")) - assert _public_groups.reconcile_moved_refs(refs) == 1 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 1 + assert "_internal.models.Parser" in moved_to assert "_internal.models.Parser" in group.owned_refs @@ -139,7 +158,9 @@ def test_reconcile_disambiguates_via_other_groups(_public_groups): group2.owned_refs.add("_internal.utils.Parser") # already owned by group2 refs = _refs_dict(_ref("Parser", "_internal/models"), _ref("Parser", "_internal/utils")) - assert _public_groups.reconcile_moved_refs(refs) == 1 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 1 + assert "_internal.models.Parser" in moved_to assert "_internal.models.Parser" in group1.owned_refs @@ -148,6 +169,266 @@ def test_reconcile_logs_when_ambiguous(_public_groups, caplog): group.owned_refs.add("_internal.old.Parser") refs = _refs_dict(_ref("Parser", "_internal/a"), _ref("Parser", "_internal/b")) - assert _public_groups.reconcile_moved_refs(refs) == 0 + count, moved_to = _public_groups.reconcile_moved_refs(refs) + assert count == 0 + assert not moved_to assert "_internal.old.Parser" in group.owned_refs # kept stale assert "Cannot resolve" in caplog.text + + +def _code_state(*refs: RefSymbol) -> PkgCodeState: + import_id_refs = {f"pkg.{r.local_id}": r for r in refs} + files = [ + PkgSrcFile( + path=Path(f"/tmp/{r.rel_path}.py"), + relative_path=r.rel_path, + pkg_import_name="pkg", + ) + for r in refs + ] + seen_paths: set[str] = set() + unique_files = [] + for f in files: + if f.relative_path not in seen_paths: + seen_paths.add(f.relative_path) + unique_files.append(f) + return PkgCodeState(pkg_import_name="pkg", import_id_refs=import_id_refs, files=unique_files) + + +def _tool_state(tmp_path: Path) -> PkgExtState: + changelog_dir = tmp_path / ".changelog" + changelog_dir.mkdir() + pkg_path = tmp_path / "pkg" + pkg_path.mkdir() + return PkgExtState(repo_root=tmp_path, changelog_dir=changelog_dir, pkg_path=pkg_path) + + +def test_named_refs_preserves_duplicate_short_names(): + """Two functions named init_cmd in different modules both appear in named_refs.""" + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + + named = code.named_refs + assert len(named) == 2 + local_ids = set(named.keys()) + assert "_internal.core.cmd_init.init_cmd" in local_ids + assert "_internal.config.cmd_config.init_cmd" in local_ids + + +def test_has_decision_distinguishes_same_name(tmp_path): + """has_decision for one init_cmd must not block the other.""" + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="init_cmd", group="core", full_path="_internal.core.cmd_init.init_cmd", author="test") + ) + assert state.has_decision("_internal.core.cmd_init.init_cmd") + assert not state.has_decision("_internal.config.cmd_config.init_cmd") + + +def test_added_refs_includes_both_same_name(tmp_path): + """Both init_cmd refs appear as added when neither has a decision.""" + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + state = _tool_state(tmp_path) + + added = state.added_refs(code.named_refs) + assert len(added) == 2 + + +def test_added_refs_filters_only_decided(tmp_path): + """After deciding one init_cmd, the other still appears as added.""" + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="init_cmd", group="core", full_path="_internal.core.cmd_init.init_cmd", author="test") + ) + + added = state.added_refs(code.named_refs) + assert len(added) == 1 + assert "_internal.config.cmd_config.init_cmd" in added + + +def test_keep_private_does_not_shadow_other_same_name(tmp_path): + """keep_private for one ref must not block the other from appearing as added.""" + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + state = _tool_state(tmp_path) + state.update_state(KeepPrivateAction(name="init_cmd", full_path="_internal.core.cmd_init.init_cmd", author="test")) + + added = state.added_refs(code.named_refs) + assert len(added) == 1 + assert "_internal.config.cmd_config.init_cmd" in added + + +def test_ref_symbol_lookup_by_local_id(): + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + + found = code.ref_symbol("_internal.core.cmd_init.init_cmd") + assert found.rel_path == "_internal/core/cmd_init" + + found2 = code.ref_symbol("_internal.config.cmd_config.init_cmd") + assert found2.rel_path == "_internal/config/cmd_config" + + # import_id key lookup (first branch in ref_symbol) + found3 = code.ref_symbol(f"pkg.{core_ref.local_id}") + assert found3.rel_path == "_internal/core/cmd_init" + + with pytest.raises(RefSymbolNotInCodeError): + code.ref_symbol("nonexistent_func") + + +def test_exposed_refs_keys_by_local_id(tmp_path): + ref_a = _ref("my_func", "_internal/mod_a") + code = _code_state(ref_a) + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="my_func", group="grp", full_path="_internal.mod_a.my_func", author="test") + ) + exposed = state.exposed_refs("grp", code.named_refs) + assert "_internal.mod_a.my_func" in exposed + + +def test_exposed_refs_preserves_both_same_name(tmp_path): + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="init_cmd", group="grp", full_path="_internal.core.cmd_init.init_cmd", author="test") + ) + state.update_state( + MakePublicAction(name="init_cmd", group="grp", full_path="_internal.config.cmd_config.init_cmd", author="test") + ) + exposed = state.exposed_refs("grp", code.named_refs) + assert len(exposed) == 2 + assert "_internal.core.cmd_init.init_cmd" in exposed + assert "_internal.config.cmd_config.init_cmd" in exposed + + +def test_ref_symbol_raises_on_ambiguous_short_name(): + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + + with pytest.raises(RefSymbolNotInCodeError, match="ambiguous"): + code.ref_symbol("init_cmd") + + +def test_ref_symbol_unique_short_name_resolves(): + ref_a = _ref("my_func", "_internal/mod_a") + code = _code_state(ref_a) + found = code.ref_symbol("my_func") + assert found.rel_path == "_internal/mod_a" + + +def test_code_ref_resolves_via_owned_refs(tmp_path): + core_ref = _ref("init_cmd", "_internal/core/cmd_init") + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(core_ref, config_ref) + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="init_cmd", group="core", full_path="_internal.core.cmd_init.init_cmd", author="test") + ) + state.update_state( + MakePublicAction( + name="init_cmd", group="config", full_path="_internal.config.cmd_config.init_cmd", author="test" + ) + ) + result = state.code_ref(code, "core", "init_cmd") + assert result is not None + assert result.rel_path == "_internal/core/cmd_init" + + result2 = state.code_ref(code, "config", "init_cmd") + assert result2 is not None + assert result2.rel_path == "_internal/config/cmd_config" + + +def test_removed_refs_fallback_without_group(tmp_path): + """Legacy ref without group info: short-name fallback keeps it when any same-name ref exists.""" + state = _tool_state(tmp_path) + state.refs["init_cmd"] = RefState(name="init_cmd", type=RefStateType.EXPOSED) + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(config_ref) + + removed = state.removed_refs(code) + assert len(removed) == 0 + + +def test_reconcile_with_code_covers_moved_symbol(tmp_path): + """After a symbol moves, reconcile_with_code prevents re-flagging as new.""" + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="check_cmd", group="core", full_path="_internal.core.cmd_check.check_cmd", author="test") + ) + assert state.has_decision("_internal.core.cmd_check.check_cmd") + assert not state.has_decision("_internal.check.cmd_check.check_cmd") + + new_ref = _ref("check_cmd", "_internal/check/cmd_check") + code = _code_state(new_ref) + state.reconcile_with_code(code.import_id_refs) + assert state.has_decision("_internal.check.cmd_check.check_cmd") + + added = state.added_refs(code.named_refs) + assert len(added) == 0 + + +def test_reconcile_with_code_groups_yaml_already_has_correct_ref(tmp_path): + """When .groups.yaml already has the correct path, the moved-to ref still enters decided_local_ids.""" + state = _tool_state(tmp_path) + # Changelog replays with stale full_path + state.update_state( + MakePublicAction(name="check_cmd", group="core", full_path="_internal.core.cmd_check.check_cmd", author="test") + ) + # Simulate .groups.yaml already containing the correct ref (e.g., manually fixed or from a prior run) + grp = state.groups.name_to_group["core"] + grp.owned_refs.discard("_internal.core.cmd_check.check_cmd") + grp.owned_refs.add("_internal.check.cmd_check.check_cmd") + + new_ref = _ref("check_cmd", "_internal/check/cmd_check") + code = _code_state(new_ref) + state.reconcile_with_code(code.import_id_refs) + + assert state.has_decision("_internal.check.cmd_check.check_cmd") + assert len(state.added_refs(code.named_refs)) == 0 + + +def test_reconcile_with_code_no_move_leaves_decisions_unchanged(tmp_path): + """When nothing moves, no new decisions are added from owned_refs.""" + state = _tool_state(tmp_path) + grp = state.groups.get_or_create_group("core") + grp.owned_refs.add("_internal.core.cmd_init.init_cmd") + + ref = _ref("init_cmd", "_internal/core/cmd_init") + code = _code_state(ref) + state.reconcile_with_code(code.import_id_refs) + + assert not state.has_decision("_internal.core.cmd_init.init_cmd") + + +def test_removed_refs_detects_specific_deletion(tmp_path): + """When core.init_cmd is deleted but config.init_cmd remains, only core is removed.""" + state = _tool_state(tmp_path) + state.update_state( + MakePublicAction(name="init_cmd", group="core", full_path="_internal.core.cmd_init.init_cmd", author="test") + ) + state.update_state( + MakePublicAction( + name="init_cmd", group="config", full_path="_internal.config.cmd_config.init_cmd", author="test" + ) + ) + # Only config.init_cmd remains in code + config_ref = _ref("init_cmd", "_internal/config/cmd_config") + code = _code_state(config_ref) + + removed = state.removed_refs(code) + assert len(removed) == 1 + group, ref_state = removed[0] + assert group == "core" + assert ref_state.name == "init_cmd" diff --git a/pkg_ext/_internal/pkg_state.py b/pkg_ext/_internal/pkg_state.py index 64dc6dd..fddf266 100644 --- a/pkg_ext/_internal/pkg_state.py +++ b/pkg_ext/_internal/pkg_state.py @@ -24,7 +24,7 @@ from pkg_ext._internal.models.groups import PublicGroups from pkg_ext._internal.models.py_symbols import RefSymbol from pkg_ext._internal.models.ref_state import RefState, RefStateType, RefStateWithSymbol -from pkg_ext._internal.models.types import qualified_name, ref_id_module +from pkg_ext._internal.models.types import qualified_name, ref_id_module, ref_id_name class PkgExtState(Entity): @@ -39,6 +39,10 @@ class PkgExtState(Entity): default_factory=PublicGroups, description="Use with caution, inferred by changelog_dir entries.", ) + decided_local_ids: set[str] = Field( + default_factory=set, + description="local_ids that have a make_public or keep_private decision", + ) ignored_shas: set[str] = Field( default_factory=set, description="Fix commits not included in the changelog", @@ -68,6 +72,11 @@ def code_ref(self, code_state: PkgCodeState, group: str, name: str) -> RefSymbol key = qualified_name(group, name) if state := self.refs.get(key): if state.exist_in_code: + if grp := self.groups.name_to_group.get(group): + for owned_ref in grp.owned_refs: + if owned_ref.endswith(f".{name}"): + with suppress(RefSymbolNotInCodeError): + return code_state.ref_symbol(owned_ref) with suppress(RefSymbolNotInCodeError): return code_state.ref_symbol(name) return None @@ -105,12 +114,16 @@ def _handle_make_public(self, name: str, group: str, full_path: str) -> None: grp = self.groups.get_or_create_group(group) grp.owned_refs.add(full_path) grp.owned_modules.add(ref_id_module(full_path)) + self.decided_local_ids.add(full_path) def _handle_keep_private(self, name: str, full_path: str) -> None: - if state := self.refs.get(name): + key = full_path or name + if state := self.refs.get(key): state.type = RefStateType.HIDDEN else: - self.refs[name] = RefState(name=name, type=RefStateType.HIDDEN) + self.refs[key] = RefState(name=name, type=RefStateType.HIDDEN) + if full_path: + self.decided_local_ids.add(full_path) for grp in self.groups.groups: grp.owned_refs.discard(full_path) @@ -177,35 +190,45 @@ def is_group_ga(self, group: str) -> bool: def get_deprecation_replacement(self, key: str) -> str: return self.deprecation_replacements.get(key, "") - def _refs_by_short_name(self) -> dict[str, list[RefState]]: - """Group refs by short name for lookups when group is unknown.""" - from collections import defaultdict - - result: dict[str, list[RefState]] = defaultdict(list) - for state in self.refs.values(): - result[state.name].append(state) - return result - - def has_decision(self, ref_name: str) -> bool: - """Check if any decision (expose/hide) has been made for this short name.""" - return any(state.type != RefStateType.UNSET for state in self._refs_by_short_name().get(ref_name, [])) + def reconcile_with_code(self, import_id_refs: dict[str, RefSymbol]) -> int: + """Run reconcile_moved_refs and update decided_local_ids for moved symbols.""" + count, moved_to = self.groups.reconcile_moved_refs(import_id_refs) + if moved_to: + self.decided_local_ids.update(moved_to) + all_owned = {ref for group in self.groups.groups for ref in group.owned_refs} + stale_ids = self.decided_local_ids - all_owned + if stale_ids: + owned_by_name: dict[str, set[str]] = {} + for ref_id in all_owned: + owned_by_name.setdefault(ref_id_name(ref_id), set()).add(ref_id) + for stale_id in stale_ids: + if current := owned_by_name.get(ref_id_name(stale_id)): + self.decided_local_ids.update(current) + return count + + def has_decision(self, local_id: str) -> bool: + return local_id in self.decided_local_ids def removed_refs(self, code: PkgCodeState) -> list[tuple[str, RefState]]: - """Returns list of (group, RefState) for removed refs.""" - named_refs = code.named_refs + active_local_ids = {ref.local_id for ref in code.import_id_refs.values()} result: list[tuple[str, RefState]] = [] for key, state in self.refs.items(): if state.type not in {RefStateType.EXPOSED, RefStateType.DEPRECATED}: continue - if state.name in named_refs: - continue - # Extract group from qualified_name key group = key.rsplit(".", 1)[0] if "." in key else "" + if grp := self.groups.name_to_group.get(group): + owned_for_name = {r for r in grp.owned_refs if r.endswith(f".{state.name}")} + if owned_for_name & active_local_ids: + continue + elif state.name in {ref.name for ref in code.import_id_refs.values()}: + # Fallback for legacy refs without group info: any symbol with the + # same short name still in code prevents marking as removed. + continue result.append((group, state)) return result def added_refs(self, active_refs: dict[str, RefStateWithSymbol]) -> dict[str, RefStateWithSymbol]: - return {ref_name: ref_symbol for ref_name, ref_symbol in active_refs.items() if not self.has_decision(ref_name)} + return {local_id: ref for local_id, ref in active_refs.items() if not self.has_decision(local_id)} def add_changelog_actions(self, actions: list[ChangelogAction]) -> None: assert actions, "must add at least one action" @@ -219,7 +242,9 @@ def is_exposed(self, group: str, ref_name: str) -> bool: return False def exposed_refs(self, group: str, active_refs: dict[str, RefStateWithSymbol]) -> dict[str, RefSymbol]: - return {name: state.symbol for name, state in active_refs.items() if self.is_exposed(group, name)} + return { + state.symbol.local_id: state.symbol for state in active_refs.values() if self.is_exposed(group, state.name) + } def is_pkg_relative(self, rel_path: str) -> bool: pkg_rel_path = self.pkg_path.relative_to(self.repo_root) diff --git a/pkg_ext/_internal/reference_handling/added.py b/pkg_ext/_internal/reference_handling/added.py index 67d8e62..597e372 100644 --- a/pkg_ext/_internal/reference_handling/added.py +++ b/pkg_ext/_internal/reference_handling/added.py @@ -71,7 +71,7 @@ def _expose_function_args( for func_ref, arg_refs in args_exposed.items(): arg_refs_all.extend(arg_refs) for ref in arg_refs: - if tool_state.has_decision(ref.name): + if tool_state.has_decision(ref.local_id): continue _expose_ref( ctx, @@ -211,7 +211,8 @@ def handle_added_refs(ctx: pkg_ctx) -> None: ) all_refs_decided = relevant_refs + extra_refs_decided for ref in all_refs_decided: - added_refs.pop(ref.name, None) + local_id = ref.local_id if isinstance(ref, RefSymbol) else ref.symbol.local_id + added_refs.pop(local_id, None) task.update(advance=len(all_refs_decided)) if added_refs: remaining_str = "\n".join(str(ref) for ref in added_refs.values()) diff --git a/pkg_ext/_internal/reference_handling/removed.py b/pkg_ext/_internal/reference_handling/removed.py index a12150e..803b960 100644 --- a/pkg_ext/_internal/reference_handling/removed.py +++ b/pkg_ext/_internal/reference_handling/removed.py @@ -23,18 +23,17 @@ def process_reference_renames( ) -> set[str]: """Process renames. Returns set of old names that were renamed.""" renamed_names: set[str] = set() - used_active: set[str] = set() + used_local_ids: set[str] = set() for group, ref in renames: - rename_choices = [state for name, state in active_refs.items() if name not in used_active] + rename_choices = [state for lid, state in active_refs.items() if lid not in used_local_ids] new_ref = select_ref( f"Select new name for the reference {ref.name} with type: {ref.type}", rename_choices, ) - new_name = new_ref.name - used_active.add(new_name) + used_local_ids.add(new_ref.symbol.local_id) if confirm_create_alias(ref, new_ref): raise NotImplementedError("Alias creation is not implemented yet") - ctx.add_changelog_action(RenameAction(name=new_name, group=group, old_name=ref.name)) + ctx.add_changelog_action(RenameAction(name=new_ref.name, group=group, old_name=ref.name)) renamed_names.add(ref.name) task.update(advance=1) return renamed_names