-
Notifications
You must be signed in to change notification settings - Fork 16
feat/CUS-9891-Added class to highlight the spelling mistakes in the screen #294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat/CUS-9891-Added class to highlight the spelling mistakes in the screen #294
Conversation
📝 WalkthroughWalkthroughVersion bumped from 1.0.0 to 1.0.8. New addon class introduces spell-checking for web screens via OCR text extraction, AI validation with GPT-4o, and visual highlighting of misspelled words with results uploaded to presigned S3 URLs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as WebAction Executor
participant Screenshot as Screenshot Capture
participant OCR as OCR Engine
participant AI as GPT-4o Model
participant Parser as Response Parser
participant ImgProc as Image Processor
participant S3 as S3 Storage
Client->>Screenshot: Capture current screen
Screenshot-->>Client: Screenshot file
Client->>OCR: Extract text points from screenshot
OCR-->>Client: List of OCRTextPoint objects
Client->>Client: Aggregate text from points
rect rgba(100, 150, 220, 0.2)
Note over Client,AI: Spell-Check Validation Phase
Client->>AI: Invoke GPT-4o with aggregated text
AI-->>Client: Response (PASS/FAIL with corrections)
end
Client->>Parser: Parse AI response for misspellings
Parser-->>Client: List of misspelled words
rect rgba(150, 200, 150, 0.2)
Note over Client,ImgProc: Highlighting Phase
Client->>Client: Calculate bounding boxes for misspellings
Client->>ImgProc: Highlight words on screenshot
ImgProc-->>Client: Highlighted screenshot file
end
Client->>S3: Upload highlighted screenshot via presigned URL
S3-->>Client: Upload confirmation
Client-->>Client: Return success result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (7)
spell_check/pom.xml (1)
9-9: Unusual version jump from 1.0.0 to 1.0.8.Skipping 7 minor versions (1.0.1 through 1.0.7) is unconventional. If this follows your team's versioning strategy, please disregard. Otherwise, consider incrementing to 1.0.1 for the next release.
spell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java (6)
312-323: Make the inner class static and consider using immutable fields.
WordBoundingBoxdoesn't access the outer class instance, so it should bestatic. Additionally, consider making fieldsfinalfor immutability, or use Java records (Java 14+) for cleaner syntax.🔎 Proposed refactor
- private static class WordBoundingBox { - int x1, y1, x2, y2; - String word; - - WordBoundingBox(int x1, int y1, int x2, int y2, String word) { - this.x1 = x1; - this.y1 = y1; - this.x2 = x2; - this.y2 = y2; - this.word = word; - } - } + private static class WordBoundingBox { + final int x1, y1, x2, y2; + final String word; + + WordBoundingBox(int x1, int y1, int x2, int y2, String word) { + this.x1 = x1; + this.y1 = y1; + this.x2 = x2; + this.y2 = y2; + this.word = word; + } + }Or, if Java 14+ is available:
private record WordBoundingBox(int x1, int y1, int x2, int y2, String word) {}
502-506: Browser window dimensions retrieved late in the flow.JavaScript execution to get
window.innerHeightandwindow.innerWidthhappens during image highlighting. This creates an unexpected dependency on the browser state at that point. Consider retrieving these dimensions earlier in theexecute()method and passing them as parameters to keep the highlighting logic more isolated.
70-91: Consider externalizing the AI prompt template.The base prompt is a lengthy multi-line string embedded in the code. If prompt engineering requires iteration, consider externalizing it to a resource file (e.g.,
prompts/spell-check.txt) or a configurable property for easier maintenance and testing.
555-579: Consider validating the S3 presigned URL.The method uploads to
s3SignedURLwithout validating its format or origin. While the URL comes fromtestStepResult.getScreenshotUrl(), adding basic validation (e.g., checking it starts withhttps://and contains expected S3 patterns) could prevent unexpected behavior if the URL is malformed.
1-581: Consider adding unit tests for helper methods.The addon has complex logic with multiple integration points (AI, OCR, S3, Selenium) and edge cases. Unit tests for methods like
extractMisspelledWords(),calculateWordBoundingBox(), andconvertWindowToScreenshotCoordinates()would improve maintainability and catch regressions.
164-178: The model identifier "gpt-4o" is valid and current as of December 2025.Consider making the AI model configurable via a property or annotation parameter rather than hardcoding it. This improves flexibility for different deployment scenarios and allows easy updates when OpenAI releases newer models.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spell_check/pom.xmlspell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java
🔇 Additional comments (3)
spell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java (3)
448-480: Coordinate conversion logic looks correct.The method properly scales OCR window coordinates to screenshot dimensions using relative positioning, applies padding, enforces minimum dimensions, and validates boundaries. The defensive checks at lines 451-454 and 474-477 prevent invalid coordinates.
93-122: Main execution flow is well-structured.The
execute()method follows a clear sequence: capture → extract → validate → process → respond. Error handling is appropriate, and early returns for edge cases (no text found) prevent unnecessary AI calls. The separation into helper methods maintains readability.
243-307: Robust regex parsing with multiple fallback patterns.The implementation handles varied AI response formats well with primary and fallback patterns. The false-positive filtering (lines 259-262) is a good addition. However, note the alignment assumption concern raised in an earlier comment regarding line 259.
| for (int i = 0; i < words.length; i++) { | ||
| String word = words[i].trim().replaceAll("[\"']", ""); | ||
| if (word.isEmpty()) { | ||
| continue; | ||
| } | ||
|
|
||
| String lowerWord = word.toLowerCase(); | ||
| if (i < corrections.size() && lowerWord.equals(corrections.get(i))) { | ||
| logger.info("Filtered out false positive: '" + word + "' (correction is the same)"); | ||
| continue; | ||
| } | ||
|
|
||
| misspelledWords.add(lowerWord); | ||
| if (i < corrections.size()) { | ||
| logger.info("Added misspelled word: '" + word + "' (correction: '" + corrections.get(i) + "')"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array alignment assumption between misspelled words and corrections.
The code assumes that corrections.get(i) corresponds to words[i] by index position. If the AI returns mismatched list lengths or reorders items, this logic will incorrectly filter words or fail to detect false positives.
Consider matching words to corrections by context or explicit pairing rather than relying on index alignment.
🤖 Prompt for AI Agents
In
spell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java
around lines 252 to 268, the loop assumes corrections.get(i) aligns with
words[i]; change this to match by value instead of index: normalize both word
and corrections (trim, remove quotes, toLowerCase), build a Set or Map of
normalized corrections before the loop, then check membership (or lookup by
word) to decide whether to treat the word as a false positive or misspelled; log
the matched correction when found and avoid using corrections.get(i) by index so
mismatched lengths or order won't cause incorrect filtering.
| private WordBoundingBox findPartialMatch(List<OCRTextPoint> textPoints, String misspelledWord) { | ||
| String lowerMisspelled = misspelledWord.toLowerCase(); | ||
|
|
||
| for (OCRTextPoint textPoint : textPoints) { | ||
| String text = textPoint.getText(); | ||
| if (text == null || text.trim().isEmpty() || text.length() > misspelledWord.length() * 2) { | ||
| continue; | ||
| } | ||
|
|
||
| int textPointWidth = textPoint.getX2() - textPoint.getX1(); | ||
| int textPointHeight = textPoint.getY2() - textPoint.getY1(); | ||
| if (textPointWidth > MAX_TEXT_POINT_WIDTH || textPointHeight > MAX_TEXT_POINT_HEIGHT) { | ||
| continue; | ||
| } | ||
|
|
||
| String lowerText = text.toLowerCase().trim(); | ||
| int wordIndex = lowerText.indexOf(lowerMisspelled); | ||
| if (wordIndex >= 0) { | ||
| WordBoundingBox wordBox = calculateWordBoundingBox(textPoint, text, misspelledWord, wordIndex); | ||
| if (wordBox != null) { | ||
| logger.info("Close match found for '" + misspelledWord + "' in text: '" + text + | ||
| "' coords: x1=" + textPoint.getX1() + ", y1=" + textPoint.getY1()); | ||
| return wordBox; | ||
| } | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heuristic may skip valid partial matches.
Line 384 skips text blocks where text.length() > misspelledWord.length() * 2. This could miss valid occurrences of misspelled words in moderately longer text. For example, if a misspelled word is "test" (4 characters) and appears in "testing 123" (11 characters), it would be skipped since 11 > 8.
If this heuristic is intentional to reduce false positives, consider documenting it. Otherwise, adjust the multiplier or remove this constraint.
🤖 Prompt for AI Agents
In
spell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java
around lines 379 to 406, the heuristic skipping text blocks where text.length()
> misspelledWord.length() * 2 can wrongly ignore valid partial matches (e.g.,
"test" inside "testing 123"); remove or relax this hard multiplier check and
instead use a safer rule: either drop the length-based skip entirely or replace
it with a configurable maxTextPointLength (or a more permissive multiplier like
3) and document the choice; ensure you keep the other size guard
(MAX_TEXT_POINT_WIDTH/HEIGHT) and add a short comment explaining the rationale
for the new limit or make it a constant with a descriptive name so behavior is
clear.
| private WordBoundingBox calculateWordBoundingBox(OCRTextPoint textPoint, String fullText, String word, int wordStartIndex) { | ||
| int x1 = textPoint.getX1(); | ||
| int y1 = textPoint.getY1(); | ||
| int x2 = textPoint.getX2(); | ||
| int y2 = textPoint.getY2(); | ||
|
|
||
| int totalWidth = x2 - x1; | ||
| int textLength = fullText.length(); | ||
|
|
||
| if (textLength == 0) { | ||
| return null; | ||
| } | ||
|
|
||
| if (word.length() >= textLength - 2) { | ||
| return new WordBoundingBox(x1, y1, x2, y2, word); | ||
| } | ||
|
|
||
| double charWidth = (double) totalWidth / textLength; | ||
| int wordX1 = x1 + (int) (wordStartIndex * charWidth); | ||
| int wordX2 = wordX1 + (int) (word.length() * charWidth); | ||
|
|
||
| wordX1 = Math.max(x1, wordX1); | ||
| wordX2 = Math.min(x2, wordX2); | ||
|
|
||
| if (wordX2 - wordX1 < MIN_HIGHLIGHT_WIDTH) { | ||
| int center = (wordX1 + wordX2) / 2; | ||
| wordX1 = Math.max(x1, center - MIN_HIGHLIGHT_WIDTH / 2); | ||
| wordX2 = Math.min(x2, center + MIN_HIGHLIGHT_WIDTH / 2); | ||
| } | ||
|
|
||
| return new WordBoundingBox(wordX1, y1, wordX2, y2, word); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounding box calculation assumes fixed character width.
Lines 428-430 calculate character width as totalWidth / textLength, which assumes monospaced fonts. Most web fonts are proportional, so this will misalign highlights for words containing wide characters (e.g., "W", "M") or narrow ones (e.g., "i", "l").
This is a known limitation without detailed font metrics from OCR. Consider documenting this caveat or investigating if the OCR service provides character-level bounding boxes.
🤖 Prompt for AI Agents
In
spell_check/src/main/java/com/testsigma/addons/web/HighlightSpellingMistakesInTheCurrentScreen.java
around lines 411 to 442, the current bounding-box calculation assumes fixed
character width by dividing totalWidth by fullText.length(), which misaligns
highlights for proportional fonts; update the method to first check whether the
OCRTextPoint (or OCR response) provides character- or glyph-level bounding boxes
and, if available, compute wordX1/wordX2 from those character boxes for exact
placement; if per-character boxes are not available, document the limitation in
a comment and replace the current strict per-char width logic with a safer
fallback (e.g., use the original line bounding box for the word or expand the
highlight to a minimum width while logging a debug/warning that proportional
fonts may cause misalignment) so callers know the caveat and highlights are less
likely to be mispositioned.
Publish this addon as public
Addon Name: spell check
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira : https://testsigma.atlassian.net/browse/CUS-9891
Added class to highlight the spelling mistakes in the screen
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.