Refactor(packages/app/src/components/prompt-input/history.ts):115 - #119
Open
jana-hussien wants to merge 2 commits into
Open
jana-hussien wants to merge 2 commits into
jana-hussien wants to merge 2 commits into
Conversation
…ion with high complexity (count = 42)
isPromptEqual had a length check, two manual for-loops,
and a separate if-check for every prompt part type. Qlty flagged it
for high complexity (count = 42).
I pulled the per-type comparison logic out into PART_COMPARATORS, an
object with one small function per part type ("text", "file", "agent",
"image"). arePromptPartsEqual just looks up the right function by
partA.type and calls it, instead of walking through an if-chain that
checked types.length. File parts also needed to compare an optional
selection object field by field, so that logic became its own
isFileSelectionEqual helper rather than living inline inside the
"file" branch. With the per-part checks factored out, the two manual
for-loops (one over prompt parts, one over comments) could each be
replaced by a single Array.every call, since all they were doing was
checking every element pairwise and bailing on the first mismatch.
Added tests for deduping file/agent/image parts, mismatched
selections, mismatched part types, and mismatched comments, since the
old tests only covered the text-part case.
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
1 participant
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.
Issue for this PR
Closes #23
Type of change
What does this PR do?
isPromptEqual()inpackages/app/src/components/prompt-input/history.tswas doing too much in one place (i.e. checking lengths, looping over prompt parts by hand, and running a big if/else chain to compare text vs file vs agent vs image parts). Qlty flagged it as high complexity (42) at line 115.To fix it I split the per-type equality checks out of that if/else chain and into their own small functions, one per part type, collected in an object called
PART_COMPARATORS. So instead ofisPromptEqualchecking "is this a text part? is this a file part? is this an agent part?" every time, it just looks up the right function forpartA.typeand runs it. The file-selection comparison (checking start/end line and char) was its own chunk of logic buried inside the file-part branch, so that became its ownisFileSelectionEqualfunction too.Once the per-type checks were pulled out, the two for-loops in
isPromptEqual(one looping over prompt parts, one over comments) didn't really need to be loops anymore. They were just checking "does every element match the one at the same index," which is whatArray.everydoes, so I replaced them with that.I didn't change what counts as "equal" for any part type, just how the checks are organized, so the function should behave the same as before.
How did you verify your code works?
I ran the tests that already existed for this file, then added a few more for cases they didn't cover: deduping file/agent/image parts, a file part with a different selection, mismatched part types, and mismatched comment fields, since the old tests only really exercised the text-part path. Everything passes. I also ran
bun lintandtsgo -band both came back clean, and I re-ranqlty smellson the file before and after the change. The message about high-complexity onisPromptEqualis gone afterward.Screenshots / recordings
Smell before:
Smell after:
Bun lint and test:
Note: history.ts is at 100% function coverage here, and the only uncovered lines (174–253) are inside navigatePromptHistory, which I didn't touch as it is not part of the smell I targeted
Checklist
If you do not follow this template your PR will be automatically rejected.