BUG-HUNT: [consistency] Refactor complex map-building functions in ACMS #2568

New issue

Open

opened 2026-04-03 18:56:16 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 18:56:16 +00:00

Owner

Metadata

Branch: refactor/acms-map-building-functions
Commit Message: refactor(acms): simplify complex map-building functions in detail_level_maps and vocabulary
Milestone: v3.7.0
Parent Epic: #396

Background and Context

The build_effective_map function in src/cleveragents/acms/uko/detail_level_maps.py and the build_detail_level_map function in src/cleveragents/acms/uko/vocabulary.py are overly complex. They use a tagged list and a stable sort to insert new levels into the map, which is not immediately obvious and makes the code harder to read and maintain. These functions are core to the ACMS module and are called frequently, making their readability and correctness critical.

Current Behavior

The map-building functions use an indirect approach — building a tagged list and relying on a stable sort — to insert new levels into the map. While functionally correct, this pattern is non-idiomatic and obscures the intent of the code, increasing the cognitive load for contributors and the risk of introducing regressions during future modifications.

Affected locations:

src/cleveragents/acms/uko/detail_level_maps.py — build_effective_map (lines 158–201)
src/cleveragents/acms/uko/vocabulary.py — build_detail_level_map (lines 288–344)

Example of current complexity:

# detail_level_maps.py
def build_effective_map(
    parent: DetailLevelMap,
    insertions: list[tuple[str, str]] | tuple[tuple[str, str], ...],
) -> tuple[tuple[str, int], ...]:
    # Start with parent's effective levels sorted by depth
    parent_levels = parent.effective_levels()
    sorted_levels = sorted(parent_levels.items(), key=lambda x: x[1])

    # Build ordered list of level names with O(1) membership set
    ordered_names: list[str] = [name for name, _depth in sorted_levels]
    known: set[str] = set(ordered_names)

    # Apply insertions in order
    for after_level, new_level in insertions:
        if after_level not in known:
            raise ValueError(
                f"Level '{after_level}' not found in effective map "
                f"during insertion of '{new_level}'"
            )
        if new_level in known:
            raise ValueError(f"Duplicate level name '{new_level}' during insertion")
        idx = ordered_names.index(after_level)
        ordered_names.insert(idx + 1, new_level)
        known.add(new_level)

    # Reassign consecutive integers
    return tuple((name, i) for i, name in enumerate(ordered_names))

Expected Behavior

The map-building functions should be refactored to be more readable and maintainable, using idiomatic Python patterns that clearly express the intent of the operation. A simpler approach would use a list of tuples or a dictionary to store the levels and sort them based on their depth, reducing cognitive overhead and the chance of errors.

Acceptance Criteria

build_effective_map in detail_level_maps.py is refactored to use a clearer, more idiomatic approach
build_detail_level_map in vocabulary.py is refactored to use a clearer, more idiomatic approach
All existing behaviour is preserved (no functional changes)
All existing BDD scenarios continue to pass without modification
New BDD scenarios are added to cover any previously untested edge cases exposed during refactoring
Code complexity metrics (e.g., cyclomatic complexity) are reduced for both functions
Type annotations remain complete and correct
Docstrings are updated to reflect the new implementation approach

Supporting Information

Severity: Medium — impacts maintainability and long-term code health
Category: consistency
Reported by: ca-bug-hunter (Bug Hunting supervisor)
Related Epic: #396 (Epic: ACMS Context Pipeline)

Subtasks

Analyse build_effective_map in detail_level_maps.py and design a simpler replacement
Analyse build_detail_level_map in vocabulary.py and design a simpler replacement
Implement refactored build_effective_map with updated docstring
Implement refactored build_detail_level_map with updated docstring
Tests (Behave): Verify all existing scenarios pass unchanged
Tests (Behave): Add scenarios for any newly identified edge cases
Verify coverage >= 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (refactor(acms): simplify complex map-building functions in detail_level_maps and vocabulary), followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (refactor/acms-map-building-functions).
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass.
Coverage >= 97%.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `refactor/acms-map-building-functions` - **Commit Message**: `refactor(acms): simplify complex map-building functions in detail_level_maps and vocabulary` - **Milestone**: v3.7.0 - **Parent Epic**: #396 ## Background and Context The `build_effective_map` function in `src/cleveragents/acms/uko/detail_level_maps.py` and the `build_detail_level_map` function in `src/cleveragents/acms/uko/vocabulary.py` are overly complex. They use a `tagged` list and a stable sort to insert new levels into the map, which is not immediately obvious and makes the code harder to read and maintain. These functions are core to the ACMS module and are called frequently, making their readability and correctness critical. ## Current Behavior The map-building functions use an indirect approach — building a `tagged` list and relying on a stable sort — to insert new levels into the map. While functionally correct, this pattern is non-idiomatic and obscures the intent of the code, increasing the cognitive load for contributors and the risk of introducing regressions during future modifications. **Affected locations:** - `src/cleveragents/acms/uko/detail_level_maps.py` — `build_effective_map` (lines 158–201) - `src/cleveragents/acms/uko/vocabulary.py` — `build_detail_level_map` (lines 288–344) **Example of current complexity:** ```python # detail_level_maps.py def build_effective_map( parent: DetailLevelMap, insertions: list[tuple[str, str]] | tuple[tuple[str, str], ...], ) -> tuple[tuple[str, int], ...]: # Start with parent's effective levels sorted by depth parent_levels = parent.effective_levels() sorted_levels = sorted(parent_levels.items(), key=lambda x: x[1]) # Build ordered list of level names with O(1) membership set ordered_names: list[str] = [name for name, _depth in sorted_levels] known: set[str] = set(ordered_names) # Apply insertions in order for after_level, new_level in insertions: if after_level not in known: raise ValueError( f"Level '{after_level}' not found in effective map " f"during insertion of '{new_level}'" ) if new_level in known: raise ValueError(f"Duplicate level name '{new_level}' during insertion") idx = ordered_names.index(after_level) ordered_names.insert(idx + 1, new_level) known.add(new_level) # Reassign consecutive integers return tuple((name, i) for i, name in enumerate(ordered_names)) ``` ## Expected Behavior The map-building functions should be refactored to be more readable and maintainable, using idiomatic Python patterns that clearly express the intent of the operation. A simpler approach would use a list of tuples or a dictionary to store the levels and sort them based on their depth, reducing cognitive overhead and the chance of errors. ## Acceptance Criteria - [ ] `build_effective_map` in `detail_level_maps.py` is refactored to use a clearer, more idiomatic approach - [ ] `build_detail_level_map` in `vocabulary.py` is refactored to use a clearer, more idiomatic approach - [ ] All existing behaviour is preserved (no functional changes) - [ ] All existing BDD scenarios continue to pass without modification - [ ] New BDD scenarios are added to cover any previously untested edge cases exposed during refactoring - [ ] Code complexity metrics (e.g., cyclomatic complexity) are reduced for both functions - [ ] Type annotations remain complete and correct - [ ] Docstrings are updated to reflect the new implementation approach ## Supporting Information - **Severity**: Medium — impacts maintainability and long-term code health - **Category**: consistency - **Reported by**: ca-bug-hunter (Bug Hunting supervisor) - Related Epic: #396 (Epic: ACMS Context Pipeline) ## Subtasks - [ ] Analyse `build_effective_map` in `detail_level_maps.py` and design a simpler replacement - [ ] Analyse `build_detail_level_map` in `vocabulary.py` and design a simpler replacement - [ ] Implement refactored `build_effective_map` with updated docstring - [ ] Implement refactored `build_detail_level_map` with updated docstring - [ ] Tests (Behave): Verify all existing scenarios pass unchanged - [ ] Tests (Behave): Add scenarios for any newly identified edge cases - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`refactor(acms): simplify complex map-building functions in detail_level_maps and vocabulary`), followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`refactor/acms-map-building-functions`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97%. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-03 18:56:29 +00:00

freemo added a new dependency

2026-04-03 18:58:34 +00:00

#396 Epic: ACMS Context Pipeline

freemo referenced this issue

2026-04-03 19:00:56 +00:00

Task: Autonomous build progress report — v3.7.0 Production Push — 2026-04-03 #2436

freemo commented

2026-04-03 19:02:22 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
MoSCoW: Should Have

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: Should Have --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added a new dependency

2026-04-03 19:08:53 +00:00

#2597 fix(ci): restore all CI quality gates to passing on master — no suppression, no bypasses

freemo removed a dependency

2026-04-14 22:18:55 +00:00

#2597 fix(ci): restore all CI quality gates to passing on master — no suppression, no bypasses