Skip to content

Resolves packages/ui/src/theme/v2/mapping.ts line duplication - #42

Draft
ariaf7 wants to merge 3 commits into
CMU-313:mainfrom
ariaf7:feature/mapping
Draft

Resolves packages/ui/src/theme/v2/mapping.ts line duplication#42
ariaf7 wants to merge 3 commits into
CMU-313:mainfrom
ariaf7:feature/mapping

Conversation

@ariaf7

@ariaf7 ariaf7 commented Sep 3, 2026

Copy link
Copy Markdown

P1B: Starter Task: Refactoring PR

Use this pull request template to briefly answer the questions below in one to two sentences each.
Feel free to delete this text at the top after filling out the template.

1. Issue

Link to the associated GitHub issue:
#20
Full path to the refactored file:
packages/ui/src/theme/v2/mapping.ts
What do you think this file does?
This file is responsible for mapping token colors for both the light and dark themes of the UI. It assigns colors for text, borders, states, etc, and returns the mapping depending on whether the app is in light or dark mode.

What is the scope of your refactoring within that file?
This refactoring joined the duplicate token definitions into shared dictionaries that store both the light and dark values, and then added logic that is in charge of choosing which values from the dictionary to assign to the variables depending on which mode is being used. The regions touched were the semantic token mappings, the agent token mappings, and the construction of the final light and dark mappings used by mapV2Semantics (mapV2Semantics was not changed though)

Which Qlty‑reported issue did you address?

Found 64 lines of similar code in 2 locations (mass = 405)

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The issue was duplicated semantic token mapping, which made the file much longer than necessary, and would mean updating multiple sections if changing or adding a token needed to happen.
What changes did you make to resolve the issue?
I made one shared token structure to store both the light and dark values, and added logic to handle that combination.
How do your changes improve maintainability? Did you consider alternatives?
The refactor removes duplication which makes the values of the tokens easier to update, compare, and makes it less likely that one theme would be updated and not the other. I debated removing smaller chunks of the duplicated sections and leaving the rest of the of the structure the same, but that still meant a lot of the duplication would remain, which would defeat the purpose of the refactor.

3. Validation

How did you validate that the change is correct?
My test file mapping.test.ts compares the new mappings from mapV2Semantics against the literal original mappings from before the refactor. This ensures that the refactor kept the existing semantic token values for both light and dark themes correctly. There was also a 99.6% line coverage, with the lines that were not covered being lines I did not change, and that did not have tests beforehand.
Attach a screenshot of the test coverage showing the lines were executed by the tests.
Screenshot 2026-09-03 at 5 32 26 PM
Attach a screenshot showing the tests that cover the change passing during CI
bun test:
Screenshot 2026-09-03 at 4 26 55 PM
bun lint:
Screenshot 2026-09-03 at 4 31 41 PM

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.
Before:
Screenshot 2026-09-03 at 5 30 23 PM
After:
Screenshot 2026-09-03 at 4 44 05 PM

@ariaf7 ariaf7 changed the title Feature/mapping Resolves packages/ui/src/theme/v2/mapping.ts line duplication Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant