refactor: clean up UI/persistence/markdown and add complexity guardrails#60
Merged
Merged
Conversation
ui.js and persist.js each defined their own MAX_LEVEL, MAX_MOB_LEVEL, PARTY_SIZE, emptyMember(), and clamp(), plus inline magic numbers for the ZEM (1-500) and minutes-per-kill (0.1-60) ranges. Extract a single src/constants.js module both import from, so the input bounds can't drift out of sync. Pure, no behavior change. Adds test/constants.test.js with golden-value coverage for the bounds and the clamp/emptyMember helpers. https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
The single while-loop carried every block type inline and duplicated the
table-start predicate in two places. Extract one parser per block
(parseFence/parseHeading/parseTable/parseList/parseParagraph), each
returning { block, next }, plus shared isFence/isHeading/isListItem/
isTableStart predicates. The main loop is now a flat dispatch. Behavior is
unchanged (covered by the 17 existing markdown tests).
Also fixes a mojibake byte in the header comment (â -> em dash).
https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
refresh() was a ~120-line function rendering party rows, totals, the ZEM and consider readouts, group-bonus cells, and the constants block inline — the highest-complexity function in the repo. Split it into clearRow, renderRow, renderRows, renderTotals, renderEncounterReadout, renderConsider, renderGroupBonus, and renderConstants; refresh() is now a short orchestrator that computes con/zemOk/activeN once and delegates. Extracts the per-row footnote decision logic into a pure src/notices.js (rowNotices) so it is unit-testable outside the DOM, with golden-value coverage in test/notices.test.js. No behavior change. https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
Add complexity (max 10) and max-depth (max 4) rules for src/**, just above the current maxima, so the engine and UI can't regress into the kind of sprawl this refactor removed. Extract applyProp() from the el() DOM helper so el drops from complexity 11 to within the new bound. https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
The earlier "decompose renderMarkdown" commit only landed the comment fix; the decomposition Edit had failed silently in a batched change, leaving renderMarkdown monolithic (cyclomatic complexity 24, over the new ESLint ceiling of 10). This actually splits it into isFence/isHeading/isListItem/ isTableStart detectors and parseFence/parseHeading/parseTable/parseList/ parseParagraph parsers behind a parseBlock dispatch; renderMarkdown is now a short scan loop. Also splits persist.deserialize (complexity 17) by extracting sanitizeEnc and a finiteOr helper, bringing it and sanitizeMember under the ceiling. Behavior unchanged: 222 tests pass, lint and format clean. https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
ajbtech
changed the title
Update the in-page header title and the browser tab <title> from "EQ XP Calculator" to "Gorrek's EQ XP Calculator". https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ
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
2 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.
Codebase cleanup pass focused on Clean Code and reducing cyclomatic complexity. The pure XP engine (
race/class/hell/mob/level/group/eligibility/character/party/partyxp/split/award/kills) was already clean and is left untouched — the changes target the few real complexity/duplication hotspots:ui.js,markdown.js, and the duplication betweenui.jsandpersist.js.All changes are behavior-preserving and developed test-first per
CLAUDE.md. 210 tests pass; lint and format clean.What changed
Centralize shared bounds & primitives —
src/constants.js(new)ui.jsandpersist.jseach defined their ownMAX_LEVEL,MAX_MOB_LEVEL,PARTY_SIZE,emptyMember(), andclamp(), plus inline magic numbers for the ZEM (1–500) and minutes-per-kill (0.1–60) ranges. These now live in one module both import from, so the input bounds can't drift out of sync. Covered bytest/constants.test.js.Decompose
ui.jsrefresh()refresh()was a ~120-line function (the highest-complexity in the repo) rendering party rows, totals, the ZEM/consider readouts, group-bonus cells, and the constants block inline. Split into focused renderers (clearRow,renderRow,renderRows,renderTotals,renderEncounterReadout,renderConsider,renderGroupBonus,renderConstants);refresh()is now a short orchestrator. The per-row footnote logic moved to a pure, unit-testablesrc/notices.js(rowNotices), covered bytest/notices.test.js. Also extractedapplyProp()from theel()DOM helper.Decompose
markdown.jsrenderMarkdown()The single block loop carried every block type inline and duplicated the table-start predicate. Split into shared detectors (
isFence/isHeading/isListItem/isTableStart) and one parser per block (parseFence/parseHeading/parseTable/parseList/parseParagraph) behind aparseBlockdispatch; the main loop is now a flat scan. Also fixed a mojibake byte in the header comment (â→ em dash). Covered by the existing 17 markdown tests.Reduce
persist.jsdeserialize()complexityExtracted
sanitizeEnc()and afiniteOr()helper sodeserializeandsanitizeMembercome in under the new complexity ceiling. Covered by the existing persist tests.Enforce complexity guardrails —
eslint.config.jsAdded
complexity(max 10) andmax-depth(max 4) forsrc/**, set just above the post-refactor maxima so today's code passes but regressions fail CI — operationalizing the "minimize cyclomatic complexity" goal in CI rather than relying on review.Out of scope
consider.js(Tier 4 in the plan) is intentionally left untouched.Note for reviewers
GitHub's diff for
markdown.jsstrips the U+F8FF private-use placeholder characters, so the inline-token sentinels (`${i}`) appear to be dropped. They are present on disk and verified: a string likebuy 5 apples and `x=7`renders correctly with digits in the text left untouched.https://claude.ai/code/session_0182ZaBayGmnqjHjxKDtWdPJ