Skip to content

CodeHealth refactor: break out API adapters and frontend sortof-app into smaller modules #1

Description

@indifferentketchup

Problem Statement

Two Red Code hotspots (Code Health < 4.0) are degrading both delivery speed and defect rate:

  • api/adapters.py (2.43) — CodeScene identifies a brain method (_apply_branch_rules, 226 LoC, 5-level nesting), low cohesion (four distinct responsibilities in one file), build_response with 9 arguments, and 7-branch kind chain in _apply_curated_picker. Business case: bringing this to 5.15 gives 32–59% fewer defects and 5–25% faster changes.
  • frontend/sortof-app.jsx (3.39) — 22 top-level components/functions in one file, WarnRow cc=55 / 147 LoC, ModTable cc=43 / 128 LoC, global D Proxy pattern replacing props, and 4–5 levels deep JSX nesting in the branch picker. Business case: bringing this to 5.15 gives 17–40% fewer defects and 3–16% faster changes.

There are no tests in the repo (neither API nor frontend), so refactoring these files without first establishing a test harness is risky.

Solution

API (adapters.py): Progressive in-file refactor → extract functions into smaller functions within the same file, then split into four modules. Add pytest test coverage as part of the refactor, not after it.

Frontend (sortof-app.jsx): Extract sub-components and hooks, reduce nesting, replace the global D Proxy pattern with a lightweight context-based data provider. Add React Testing Library for the extracted components.

Commits

Phase 0: Foundation — API tests (2 commits)

Commit 0: Add pytest infrastructure and test build_warnings

  • Create api/tests/ directory with conftest.py, __init__.py, and pytest.ini
  • Add requirements-test.txt with pytest, pytest-asyncio, httpx
  • Add a test for build_warnings — a pure function with no side effects, ideal for the first test. Input: mlos_sort warnings dict. Output: SORTOF_DATA WARNINGS shape. This is the simplest adapter to test and validates the test setup works.

Commit 1: Add tests for build_response with in-memory mod data

  • Create test fixtures: a small MODS_LINE-sized set of ModInfo objects (8–12 mods covering category buckets)
  • Write tests for build_response using those fixtures: normal path (no warnings, no branches), path with a cycle warning, path with a missing-dep warning
  • These tests use pytest-mock for any Steam API or DB lookups. No real DB needed — all dependencies are mocked.

Phase 1: api/adapters.py — in-file refactor (12 commits)

Commit 2: Extract _apply_branch_rules — exemptions into separate functions

  • Create 5 private functions at the top of the file:
    • _is_obsolete_exemption(group) — drops OBSOLETE/DEPRECATED mod_ids
    • _is_curated_exemption(wsid) — checks CURATED_PICKER_RULES
    • _is_addon_exemption(group) — checks is_addon flag
    • _is_radio_exemption(group) — checks incompatibleMods self-reference
    • _is_coordinated_exemption(group, input_modids) — checks cross-refs in require/loadAfter/loadBefore
  • Replace the exemption chain (lines 828–930) with sequential calls to these functions
  • No behavioral change; just moving conditionals into named functions

Commit 3: Extract Rule A (_handle_rule_a) from _apply_branch_rules

  • Rule A is the "build-aware default" path (lines 938–970): match active vs opposite flavor
  • Extract into a function that takes the group and returns (drop_ids, hints, warnings)
  • Rule A is self-contained — it doesn't depend on later rules

Commit 4: Extract Rule C (_handle_rule_c) from _apply_branch_rules

  • Rule C is the input cross-reference path (lines 973–1058): suffix-token matching
  • Extract the suffix extraction, matching, and variant/family detection into a function
  • The variant_family / addon_like detection (lines 1035–1041) becomes a helper

Commit 5: Extract Rule B fallback (_handle_rule_b_fallback) from _apply_branch_rules

  • Rule B is the prefix-base or alphabetical-first fallback (lines 1069–1117)
  • Extract into a function that takes the group and returns (drop_ids, hints)
  • Handles multi-component pack detection (lines 1094–1117)

Commit 6: Extract the resort override path (_handle_resort_override) from _apply_branch_rules

  • User picks win over auto-pick (lines 1124–1131)
  • Extract into a small helper — this is simple logic but currently buried

Commit 7: Extract _apply_curated_pickerkind dispatch into strategy pattern

  • Replace the if/elif chain for xor/xor_optional/additive/keep_all/require_wsid/default_off/exclude with a dict dispatch:
    _KIND_HANDLERS = {
        "xor": _handle_xor,
        "xor_optional": _handle_xor_optional,
        "additive": _handle_additive,
        ...
    }
    result = _KIND_HANDLERS[rules.get("kind", "keep_all")](...)
  • Each handler returns (drop_ids, warnings, hints) — consistent return shape
  • Extract prefer_non_standalone_pairs post-processing into _handle_prefer_pairs

Commit 8: Extract _resolve_conflicts into its own module

  • This is a graph-based connected-component analysis — it doesn't depend on adapters.py's data structures
  • Move to api/conflict_resolver.py (a new file)
  • Create a resolve_conflicts(mods, sorted_order) -> (picker_member_ids, warnings) API
  • Update build_response to import from the new module

Commit 9: Extract category translation and position utilities into api/constants.py

  • CAT_MAP, to_frontend_cat, position, MOD_ID_ALIASES, _resolve_satisfied_via_alias
  • These are pure constants — no dependency on anything in adapters.py
  • Move to constants.py and update imports in adapters.py and enrichment.py

Commit 10: Extract mod_id heuristics into api/mod_id_utils.py

  • _build_flavor, _strict_prefix_of, _prefix_base, _branch_hint, _suffix_token, _longest_common_prefix_ci, _longest_common_suffix_ci, _input_match_terms
  • Plus _HINT_PATTERNS constant
  • Pure string-analysis utilities — no dependency on ModInfo

Commit 11: Extract curated picker rules into api/curated_rules.py

  • CURATED_PICKED_RULES (the big data dict), _CURATED_HARD_EXCLUDE, _CURATED_GROUP_MEMBERS, _CURATED_FORCE_CATEGORY
  • curated_category_for, curated_always_on_ids
  • The evaluation engine (_apply_curated_picker) stays in adapters.py for now — it depends on ModInfo from mlos_sort.py and the MOD_ID_ALIASES which will be in constants.py
  • This commit just extracts the data

Commit 12: Extract build_response — argument soup into a config dataclass

  • Replace the 9-argument signature with:
    @dataclass
    class BuildResponseConfig:
        sort_inputs: SortInputs  # input_ids, hit_ids, mods, sort_result, status
        enrichment: EnrichmentConfig  # wsid_lookup, pz_build
        resort: ResortConfig  # is_resort, selected_modids
  • Extract the map-line composition (lines 1320–1361) into _compose_map_line
  • Extract the warning suppression logic into _filter_suppressed_warnings

Phase 2: api/adapters.py — cleanup (4 commits)

Commit 13: Add type annotations to extracted functions

  • Add ModInfo type hints to all extracted functions
  • Add return type annotations to all public functions
  • Run py_compile to verify

Commit 14: Add docstrings to all extracted functions

  • Each extracted function needs a one-line docstring explaining what it does
  • This is important because the original file is dense with comments and extracted functions will be standalone

Commit 15: Consolidate imports

  • Clean up unused imports in adapters.py
  • Ensure constants.py, mod_id_utils.py, curated_rules.py, conflict_resolver.py each have minimal imports

Commit 16: Add tests for the extracted functions

  • Tests for _apply_branch_rules with all three rules (A, B, C) and all five exemptions
  • Tests for _apply_curated_picker with each kind
  • Tests for resolve_conflicts with a 2-mod incompatibility graph
  • Tests for build_response with all map-line composition paths

Phase 3: Frontend — component extraction (6 commits)

Commit 17: Extract WarnRow — action buttons into a sub-component

  • Create WarnRowActions — extracts the action button mapping (lines 628–677)
  • Props: actions: WarningAction[], handlers for add/remove/swap/search
  • This is a pure render component — no state, no effects
  • Add a test for WarnRowActions with each action type

Commit 18: Extract WarnRow — branch picker into a sub-component

  • Create WarnBranchPicker — extracts the deepest JSX nesting (lines 679–752)
  • Props: wsid, alternatives, isAdditive, picked, onToggle, onPick
  • Move isRadioMode and defaultSelectionForBranches into a useBranchPicker hook
  • This is the highest-impact frontend commit — eliminates the 5-level nesting

Commit 19: Extract WarnRow — alwaysOn and rowStaged into a hook

  • Create useWarnRow(w) — returns {alwaysOn: Set, rowStaged: boolean, hasPicker: boolean}
  • Extract computeAlwaysOn from the loop into a pure function
  • Extract actionStaged check into a memoized helper
  • This eliminates the O(warnings * MOD_DB) per-render pattern

Commit 20: Extract ModTable — single-branch row into a sub-component

  • Create ModRow — extracts the single-branch row (lines 1065–1084)
  • Props: mod: ModSpec, wsid: string, index: number
  • Add a test for ModRow with a single branch

Commit 21: Extract ModTable — multi-branch row into a sub-component

  • Create MultiBranchRow — extracts the multi-branch row (lines 1086–1130)
  • Props: spec: RowSpec, selected: string, onToggleExpansion: () => void
  • This eliminates the JSX nesting in the multi-branch path

Commit 22: Extract AutoFixBar — complex conditionals into helpers

  • Extract the compound &&/|| expressions into derived state via useMemo
  • Create useAutoFixBar hook — returns {canAddDeps: boolean, canFixMismatches: boolean, canRemove: boolean}
  • Replace inline ternary conditionals with boolean variables computed via useMemo

Phase 4: Frontend — data flow refactor (3 commits)

Commit 23: Replace global D Proxy with React Context for SORTOF_DATA

  • Create SortofDataContext — wraps _liveSortData in React's createContext
  • Add SortofDataProvider component — sets _liveSortData and provides the context
  • Update all components to consume via useContext(SortofDataContext) instead of reading D
  • This is the single most impactful frontend refactor: eliminates the implicit dependency pattern
  • After this change, useMemo([D.MOD_DB]) becomes useMemo([MOD_DB]) — no more Proxy-reference-no-op

Commit 24: Extract StatusStrip — state dispatcher into a hook

  • Create useStatusState — extracts the compound conditions (isInflightPartial, isTerminalDone, showWarnings, showOutputs)
  • Replace the repeated isInflightPartial || isTerminalDone pattern with a single boolean variable

Commit 25: Extract RightColumn — state dispatcher into a hook

  • Create useRightColumnDisplay — extracts the state machine dispatch logic
  • Replace the state === 'idle' && ... state === 'loading' && ... pattern with a useMemo-based switch

Phase 5: Cleanup and verification (3 commits)

Commit 26: Run CodeScene change-set scan

  • codescene analyze --branch main --repo /home/samkintop/opt/sortof
  • Document the before/after Code Health scores
  • If any file regresses, fix it

Commit 27: Add CodeScene pre-commit guard

  • Add pre-commit config or a make code-health target
  • Document the guard in CLAUDE.md or a README

Commit 28: Set file-level goals in CodeScene for the two hotspots

  • Set 2.43 → 5.0 for adapters.py
  • Set 3.39 → 5.0 for sortof-app.jsx

Decision Document

Modules to be built

  • api/constants.pyCAT_MAP, to_frontend_cat, position, MOD_ID_ALIASES, _resolve_satisfied_via_alias
  • api/mod_id_utils.py_build_flavor, _strict_prefix_of, _prefix_base, _branch_hint, _suffix_token, _longest_common_prefix_ci, _longest_common_suffix_ci, _input_match_terms, _HINT_PATTERNS
  • api/curated_rules.pyCURATED_PICKER_RULES, _CURATED_HARD_EXCLUDE, _CURATED_GROUP_MEMBERS, _CURATED_FORCE_CATEGORY
  • api/conflict_resolver.pyresolve_conflicts (graph-based connected-component analysis)
  • frontend/sortof-app.jsx — component extraction (WarnRowActions, WarnBranchPicker, useWarnRow, ModRow, MultiBranchRow, useAutoFixBar)

Interfaces to be modified

  • build_response — 9 args → 1 dataclass config
  • build_warnings — no change to interface (already clean at 5 args)
  • _apply_branch_rules — no change to interface (already 5 args, but internally refactored)
  • _apply_curated_picker — no change to interface (kind dispatch is internal)

Technical clarifications

  • build_response is called from _sort_enrich_assemble (app.py) and resort_endpoint (app.py) — both use the 9-arg call. The dataclass replacement must be backward-compatible during the transition or done atomically in one commit.
  • D Proxy is the single global data pattern — every component reads from it. Replacing with Context requires updating all 22 components, which is a lot of changes. However, the Proxy reference never changes (it's always the same Proxy object), so useMemo([D.MOD_DB]) is effectively useMemo([]) — a real React performance issue, not just a code smell.

Architectural decisions

  1. Progressive in-file → module split for the API. Extract functions first (smaller commits, easier to review), then split files once the logic is clearer.
  2. Add tests as part of the refactor, not after. The API tests go first (easier to add with pytest + httpx). The frontend goes second (React Testing Library needs to be added to the browser-based app).
  3. Replace the global D Proxy with React Context for the frontend. This is the single most impactful change for the frontend — it eliminates the implicit dependency pattern that makes every component implicitly depend on _liveSortData being set.

Schema changes

  • None for the API (no DB changes)
  • None for the frontend (no changes to the SORTOF_DATA shape)

API contracts

  • None — the API contract (SORTOF_DATA shape) is unchanged. Only the internal assembly path changes.

Testing Decisions

What makes a good test (only test external behavior, not implementation details)

  • API tests: Test build_warnings and build_response by asserting the SORTOF_DATA output shape. For build_response, the test input is a MODS_LINE-sized set of ModInfo objects; the output is the full payload dict. Assert SORTED_ORDER length, WARNINGS tags, MOD_DB mod_ids — not how the function got there.
  • Frontend tests: Test sub-components by asserting rendered output and user interaction (click → expected callback fired). For WarnRowActions, assert that clicking an "add-wsid" button fires onAddWsid. For WarnBranchPicker, assert that clicking a branch toggles its selection.

Modules to be tested

  • API: adapters.py (5 functions), conflict_resolver.py (1 function), constants.py (0 functions — it's pure data), mod_id_utils.py (3 functions: _strict_prefix_of, _suffix_token, _longest_common_prefix_ci)
  • Frontend: WarnRowActions, WarnBranchPicker, useWarnRow, ModRow, MultiBranchRow

Prior art for the tests

  • API: No prior art — there are no existing tests. The pytest + httpx setup will be new. pytest-mock for DB/Steam mocking. The sortof-data.jsx canned data serves as inspiration for fixture shapes but is in JavaScript.
  • Frontend: No prior art — there are no existing frontend tests. The Babel-standalone in-browser architecture means React Testing Library needs to run in a JSDOM environment, not in the browser. This is a new testing layer.

Out of Scope

  • api/steam_guard.py — No goals set, no hotspot, low churn. Not part of this refactor.
  • api/enrichment.py — Not a hotspot. Low churn. Could be split later.
  • frontend/tweaks-panel.jsx — Not a hotspot. Low churn. No Code Health concerns identified.
  • frontend/sortof-data.jsx — Canned data only. No Code Health concerns.
  • frontend/index.html — CSS + HTML shell. No Code Health concerns.
  • api/mlos_sort.py — The source-sort algorithm. Not a hotspot (it's the input to adapters.py). Changing this is a different project.
  • Drain workers — No Code Health concerns identified. Different domain.
  • DB schema changes — None needed. The refactor is purely code organization.

Further Notes

  • Commit 17 (WarnRowActions) is the highest-ROI frontend commit. It extracts the action button mapping from WarnRow's cc=55 into a clean component with explicit props. This is the first win for the frontend refactor.
  • Commit 18 (WarnBranchPicker) eliminates the deepest JSX nesting. After this commit, the WarnRow JSX nesting goes from 5 levels deep to 2 levels. This is a major Code Health win.
  • The D Proxy → Context transition is non-trivial. 22 components will need to be updated. This is a single commit because splitting it would leave the app in a broken state (some components would read D, others would read Context). The Context approach is a superset of the Proxy pattern — it's still global, but React's reactivity model makes it explicit.
  • CodeScene goals should be set on both hotspots after the refactor. The current goal state is empty. Setting them at 5.0 each gives a measurable target for this refactor.
  • The isInflightPartial || isTerminalDone compound pattern appears 3 times in RightColumn. This should be a single isTerminal variable computed via useMemo. This is a small change but eliminates a real pattern smell.

Metadata

Metadata

Labels

No labels
No labels

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions