feat: Download/share a “visualization report”#1982
Open
Aditya8369 wants to merge 2 commits into
Open
Conversation
|
@Aditya8369 is attempting to deploy a commit to the eshajha19's projects Team on Vercel. A member of the Team first needs to authorize it. |
❌ Deploy Preview for algo-infinity-verse failed.
|
Contributor
📝 WalkthroughWalkthroughThis PR adds a new "Algorithm Decision Tree Assistant" tool page (HTML/CSS/JS wizard with an algorithm catalog and reference library), adds export/share/compare report features to the suffix array visualizer, and reformats markup in the navbar partial and the DSA battle mode page, including a new navbar link to the new tool. ChangesAlgorithm Decision Tree Assistant
Estimated code review effort: 4 (Complex) | ~60 minutes Suffix Array Visualizer Report/Share/Compare
Estimated code review effort: 4 (Complex) | ~60 minutes Navigation and DSA Battle Mode Markup Reformatting
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WizardUI
participant DecisionTree
participant AlgorithmsCatalog
User->>WizardUI: select option
WizardUI->>DecisionTree: lookup next node
DecisionTree-->>WizardUI: next question or result_* leaf
WizardUI->>AlgorithmsCatalog: fetch algorithm by key
AlgorithmsCatalog-->>WizardUI: metadata, code snippets
WizardUI->>User: render recommendation card
sequenceDiagram
participant User
participant VisualizerUI
participant ReportGenerator
participant Clipboard
User->>VisualizerUI: click export/share button
VisualizerUI->>ReportGenerator: getRoundsSummary(state.steps)
ReportGenerator-->>VisualizerUI: JSON or HTML report
VisualizerUI->>Clipboard: copyShareLink(getShareLink())
VisualizerUI->>User: download file or copy confirmation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
pages/tools/algorithm-decision-tree/algorithm-decision-tree.cssParsing error: Unexpected token : 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
partials/navbar.html (1)
714-768: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDuplicate navigation links in the Learn dropdown.
Lines 714–737 (unchanged) and 739–768 (changed) contain the same set of links — LeetCode Sync, Learning Path Generator, Assessments, Contest Hub, Code Playground, Create Problem (Admin), and Playground — all within the same
dropdown-menu. Only "React Playground" (line 754) is genuinely new. The rest are exact duplicates that will render twice, cluttering the menu and confusing screen reader users navigating by role="menuitem" count.Remove the duplicate block (739–768) and keep only the new "React Playground" link, inserting it into the existing set at 714–737.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@partials/navbar.html` around lines 714 - 768, The Learn dropdown in navbar.html now contains a duplicated block of existing menu items, which will render the same links twice. Update the dropdown markup by removing the repeated LeetCode Sync, Learning Path Generator, Assessments, Contest Hub, Code Playground, Create Problem (Admin), and Playground entries from the later block, and keep only the new React Playground link. Place that React Playground item alongside the existing dropdown links within the same dropdown-menu structure so the menu remains a single, non-duplicated list.pages/Dsa-Battle/dsa-battle-mode.html (1)
52-203: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDuplicate navbar: hardcoded nav plus dynamically loaded partial will produce duplicate IDs and duplicate navigation landmarks.
Lines 52–201 contain a fully hardcoded
<nav>with element IDs (navLinks,darkModeToggle,menuToggle,navLoginBtn,navSignupBtn). Line 203 adds<div id="navbar-placeholder"></div>, and line 471 callsloadPartial('navbar-placeholder', '/partials/navbar.html')which injects the shared navbar partial — containing the same IDs — into the page. This results in:
- Invalid HTML: duplicate
idattributes (navLinks,navLoginBtn, etc.) breakgetElementByIdand querySelector behavior.- Duplicate nav landmark: screen readers announce two navigation regions, violating WCAG 2.2 (bypass blocks / landmark navigation).
- Double event wiring: any JS that attaches to
#darkModeToggleor#menuTogglewill bind to the first match only, leaving the second navbar non-functional.Remove the hardcoded
<nav>(lines 52–201) and rely solely on theloadPartialinjection, or remove the placeholder and keep the hardcoded nav — not both.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/Dsa-Battle/dsa-battle-mode.html` around lines 52 - 203, The page currently renders two navbars: the hardcoded `<nav>` block and the shared navbar loaded by `loadPartial('navbar-placeholder', '/partials/navbar.html')`, which creates duplicate IDs and duplicate navigation landmarks. Update `dsa-battle-mode.html` to keep only one source of truth by either removing the hardcoded navbar markup or removing the `navbar-placeholder`/partial loading path, and make sure symbols like `navLinks`, `darkModeToggle`, `menuToggle`, `navLoginBtn`, and `navSignupBtn` exist only once.Source: Coding guidelines
🧹 Nitpick comments (2)
pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js (1)
654-664: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle clipboard write rejection.
navigator.clipboard.writeText(link)can reject (denied permission, non-secure context, or unsupported browser), and the current.then()-only chain leaves the rejection unhandled with no user feedback. Add a.catch()to surface a failure state.♻️ Proposed fix
function copyShareLink() { const link = getShareLink(); - navigator.clipboard.writeText(link).then(() => { - if (el.shareStatus) { - el.shareStatus.classList.remove('hidden'); - setTimeout(() => { - el.shareStatus.classList.add('hidden'); - }, 2000); - } - }); + navigator.clipboard + .writeText(link) + .then(() => { + if (el.shareStatus) { + el.shareStatus.classList.remove('hidden'); + setTimeout(() => { + el.shareStatus.classList.add('hidden'); + }, 2000); + } + }) + .catch(() => { + if (el.shareStatus) { + el.shareStatus.textContent = 'Could not copy link automatically.'; + el.shareStatus.classList.remove('hidden'); + } + }); }As per coding guidelines: "Follow production-grade JavaScript practices including error handling, optimization, and Node.js best practices".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js` around lines 654 - 664, The copyShareLink() flow only handles the success path from navigator.clipboard.writeText(link), so clipboard failures are left unhandled. Update copyShareLink() to add explicit rejection handling on the writeText promise and surface a visible failure state to the user, while keeping the existing success path that shows el.shareStatus on success.Source: Coding guidelines
pages/tools/algorithm-decision-tree/algorithm-decision-tree.js (1)
1633-1649: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winImprove
copyCodeSnippeterror handling for production use.
navigator.clipboardmay be undefined in non-secure contexts (HTTP), causing an uncaughtTypeErrorbefore the promise chain. The.catchonly logs to console with no user feedback. As per coding guidelines, follow production-grade JavaScript practices including error handling.🛡️ Proposed fix with guard and user feedback
function copyCodeSnippet() { const textToCopy = codeContentEl.textContent; - navigator.clipboard - .writeText(textToCopy) + if (!navigator.clipboard) { + copyBtn.innerHTML = `<i class="fas fa-xmark"></i> Copy unavailable`; + setTimeout(() => { copyBtn.innerHTML = `<i class="far fa-copy"></i> Copy`; }, 1500); + return; + } + navigator.clipboard.writeText(textToCopy) .then(() => { const originalText = copyBtn.innerHTML; copyBtn.innerHTML = `<i class="fas fa-check"></i> Copied!`; copyBtn.style.color = '`#10b981`'; setTimeout(() => { copyBtn.innerHTML = originalText; copyBtn.style.color = ''; }, 1500); }) .catch((err) => { console.error('Failed to copy code: ', err); + copyBtn.innerHTML = `<i class="fas fa-xmark"></i> Copy failed`; + copyBtn.style.color = '`#ef4444`'; + setTimeout(() => { + copyBtn.innerHTML = `<i class="far fa-copy"></i> Copy`; + copyBtn.style.color = ''; + }, 1500); }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js` around lines 1633 - 1649, `copyCodeSnippet` currently assumes `navigator.clipboard.writeText` is always available and only logs failures, so update it to handle non-secure/browser-unsupported cases before calling `writeText` and provide visible user feedback on failure. Add a feature guard around the clipboard access in `copyCodeSnippet`, keep the existing success state update for `copyBtn`, and in the error path for the clipboard operation surface a user-facing message instead of only `console.error`, using the existing `codeContentEl` and `copyBtn` references to keep the behavior localized.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pages/Dsa-Battle/dsa-battle-mode.html`:
- Around line 146-174: The hardcoded Profile dropdown in dsa-battle-mode.html is
missing the auth-state hooks used by the main navbar partial, so its links won’t
show/hide correctly and there is no way to log out. Update the dropdown items in
this navbar block to match the auth gating used elsewhere by adding the same
data-auth-required and data-guest-only attributes to the relevant anchors, and
include a Logout action with the data-auth-logout behavior. If this page should
instead rely on the shared navbar, remove the duplicate hardcoded menu entirely.
- Around line 155-166: The dropdown menu in dsa-battle-mode.html has two “Coding
Resume” entries pointing to different hrefs, which creates a duplicate
navigation item. Remove the incorrect relative resume.html link from the menu
and keep the absolute /pages/career/resume/resume.html target so the dropdown
matches the navbar partial and resolves correctly from the Dsa-Battle page.
- Around line 459-469: The page script block in dsa-battle-mode.html includes
duplicate resources that can execute twice and add unnecessary requests. Update
the script tags around the existing dsa-battle-mode.js, script.js, /script.js,
/dist/legacy-bundle.mjs, and /theme.js entries so each asset is included only
once, keeping the intended order while removing the redundant duplicate
includes. Double-check the final script list to ensure the module bundle and
theme initialization are not loaded more than once.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.css`:
- Around line 207-242: Add visible :focus-visible styles for the interactive
controls in algorithm-decision-tree.css, including .adt-option-btn,
.adt-tab-btn, .adt-copy-btn, .adt-filter-btn, .adt-library-card, and
.adt-trail-step, so keyboard users get a clear focus indicator matching the
existing hover/active treatment. Update the relevant style blocks for these
selectors to include accessible focus states, and make sure the non-button
elements like .adt-library-card and .adt-trail-step are keyboard focusable by
adding tabindex="0" in the corresponding HTML/JS. Keep the changes consistent
with the existing design and WCAG 2.2 expectations.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.html`:
- Around line 206-213: The library filter buttons in the algorithm decision tree
act like toggle buttons but only update the active class, so add aria-pressed
state handling in the filterLibrary function and keep it in sync with the
current filter selection. Update the button markup/state for the existing
adt-filter-btn controls so the active button has aria-pressed="true" and the
inactive ones are false, and ensure this toggles whenever the filter changes.
- Around line 151-168: The code viewer tabs in adt-code-tabs need the full ARIA
tab pattern so screen readers can identify and operate the JavaScript/Python
toggle. Update the tab controls in the code box UI to use role="tablist" on the
container, role="tab" with aria-selected and aria-controls on each button, and
role="tabpanel" on the code panel with the matching id and aria-labelledby
relationship; also keep the active state in sync in the tab switch logic for the
adt-tab-btn and adtCodeContent elements.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js`:
- Around line 1399-1463: The transition logic in renderQuestion,
showRecommendation, and goBackStep allows repeated clicks before the new panel
finishes rendering, which can duplicate pathHistory entries. Add an interaction
lock or disable the current option/back controls as soon as a choice is made and
keep them disabled until the slide transition completes; then re-enable after
the new content is mounted. Use the existing renderQuestion, showRecommendation,
and goBackStep flow to place the guard consistently so clicks during the
200ms/250ms timeouts are ignored.
- Around line 1381-1396: The breadcrumb items in the trail rendering are
interactive but currently built as plain divs, so they are not keyboard
accessible. Update the breadcrumb markup in the trail builder logic to use
native buttons instead of adt-trail-step divs, and keep the click behavior wired
to rewindTo so Home and each pathHistory step remain operable by keyboard and
screen readers. Make sure the unique breadcrumb rendering and listener code in
the trailStepsEl/forEach block preserves the same visual structure while adding
proper accessible semantics.
- Around line 1588-1597: The problem link markup in the algorithm decision tree
is hardcoding the difficulty badge as “easy” and always showing “Practice,”
which is misleading. Update the `problemsList.innerHTML` rendering in
`algorithm-decision-tree.js` to either use a real per-problem difficulty value
from each `ALGORITHMS` entry (e.g. via `algo.problems` fields) or remove the
difficulty badge entirely if that data is not available. Make sure the change is
applied where the `prob.title`/`prob.link` anchor is generated so every problem
reflects accurate information.
In `@pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js`:
- Around line 446-638: The HTML rendering in generateHtmlReport and the
comparison UI in computeAndDisplayComparison are interpolating user-controlled
string values directly into HTML, which can lead to script injection. Add and
use a single escaping helper for every derived value from state.s, suffix
previews, metadata, and comparison output before inserting them into the report
template or any innerHTML assignment. Apply the fix consistently wherever
generateHtmlReport builds title/rows and where computeAndDisplayComparison
writes to el.compareResult.
---
Outside diff comments:
In `@pages/Dsa-Battle/dsa-battle-mode.html`:
- Around line 52-203: The page currently renders two navbars: the hardcoded
`<nav>` block and the shared navbar loaded by `loadPartial('navbar-placeholder',
'/partials/navbar.html')`, which creates duplicate IDs and duplicate navigation
landmarks. Update `dsa-battle-mode.html` to keep only one source of truth by
either removing the hardcoded navbar markup or removing the
`navbar-placeholder`/partial loading path, and make sure symbols like
`navLinks`, `darkModeToggle`, `menuToggle`, `navLoginBtn`, and `navSignupBtn`
exist only once.
In `@partials/navbar.html`:
- Around line 714-768: The Learn dropdown in navbar.html now contains a
duplicated block of existing menu items, which will render the same links twice.
Update the dropdown markup by removing the repeated LeetCode Sync, Learning Path
Generator, Assessments, Contest Hub, Code Playground, Create Problem (Admin),
and Playground entries from the later block, and keep only the new React
Playground link. Place that React Playground item alongside the existing
dropdown links within the same dropdown-menu structure so the menu remains a
single, non-duplicated list.
---
Nitpick comments:
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js`:
- Around line 1633-1649: `copyCodeSnippet` currently assumes
`navigator.clipboard.writeText` is always available and only logs failures, so
update it to handle non-secure/browser-unsupported cases before calling
`writeText` and provide visible user feedback on failure. Add a feature guard
around the clipboard access in `copyCodeSnippet`, keep the existing success
state update for `copyBtn`, and in the error path for the clipboard operation
surface a user-facing message instead of only `console.error`, using the
existing `codeContentEl` and `copyBtn` references to keep the behavior
localized.
In `@pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js`:
- Around line 654-664: The copyShareLink() flow only handles the success path
from navigator.clipboard.writeText(link), so clipboard failures are left
unhandled. Update copyShareLink() to add explicit rejection handling on the
writeText promise and surface a visible failure state to the user, while keeping
the existing success path that shows el.shareStatus on success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a5ca239-4c29-4b37-b485-883edce6aaef
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
pages/Dsa-Battle/dsa-battle-mode.htmlpages/tools/algorithm-decision-tree/algorithm-decision-tree.csspages/tools/algorithm-decision-tree/algorithm-decision-tree.htmlpages/tools/algorithm-decision-tree/algorithm-decision-tree.jspages/visualizers/suffix-array-visualizer/suffix-array-visualizer.htmlpages/visualizers/suffix-array-visualizer/suffix-array-visualizer.jspartials/navbar.html
Comment on lines
+146
to
174
| <button | ||
| type="button" | ||
| class="nav-link dropdown-toggle" | ||
| aria-haspopup="true" | ||
| aria-expanded="false" | ||
| > | ||
| <span>Profile</span><i class="fas fa-chevron-down dropdown-icon"></i> | ||
| </button> | ||
| <div class="dropdown-menu dropdown-menu-end" role="menu"> | ||
| <a href="index.html#dashboard" class="dropdown-item" role="menuitem"><i class="fas fa-tachometer-alt"></i> Dashboard</a> | ||
| <a href="resume.html" class="dropdown-item" role="menuitem"><i class="fas fa-file-alt"></i> Coding Resume</a> | ||
| <a href="/execution-history.html" class="dropdown-item" role="menuitem"><i class="fas fa-history"></i> Execution History</a> | ||
| <a href="/pages/career/resume/resume.html" class="dropdown-item" role="menuitem"><i class="fas fa-file-alt"></i> Coding Resume</a> | ||
| <a href="index.html#dashboard" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-tachometer-alt"></i> Dashboard</a | ||
| > | ||
| <a href="resume.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-file-alt"></i> Coding Resume</a | ||
| > | ||
| <a href="/execution-history.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-history"></i> Execution History</a | ||
| > | ||
| <a href="/pages/career/resume/resume.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-file-alt"></i> Coding Resume</a | ||
| > | ||
| <div class="dropdown-divider"></div> | ||
| <a href="login.html" class="dropdown-item nav-auth-link" id="navLoginBtn"><i class="fas fa-sign-in-alt"></i> Sign In</a> | ||
| <a href="signup.html" class="dropdown-item nav-auth-link" id="navSignupBtn"><i class="fas fa-user-plus"></i> Sign Up</a> | ||
| <a href="login.html" class="dropdown-item nav-auth-link" id="navLoginBtn" | ||
| ><i class="fas fa-sign-in-alt"></i> Sign In</a | ||
| > | ||
| <a href="signup.html" class="dropdown-item nav-auth-link" id="navSignupBtn" | ||
| ><i class="fas fa-user-plus"></i> Sign Up</a | ||
| > | ||
| </div> |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Profile dropdown missing auth-gating attributes and Logout button.
The navbar partial (navbar.html lines 826–858) applies data-auth-required to Dashboard, My Profile, Execution History, Coding Resume, and Settings; data-guest-only to Sign In, Sign Up, and Forgot Password; and includes a Logout button with data-auth-logout. This hardcoded dropdown has none of these attributes and no Logout button, so auth-state visibility logic won't apply and users cannot log out from this page.
If the hardcoded nav is removed per the duplicate-navbar fix above, this becomes moot. If it stays, add the corresponding data-auth-required / data-guest-only attributes and a Logout button to match the partial.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/Dsa-Battle/dsa-battle-mode.html` around lines 146 - 174, The hardcoded
Profile dropdown in dsa-battle-mode.html is missing the auth-state hooks used by
the main navbar partial, so its links won’t show/hide correctly and there is no
way to log out. Update the dropdown items in this navbar block to match the auth
gating used elsewhere by adding the same data-auth-required and data-guest-only
attributes to the relevant anchors, and include a Logout action with the
data-auth-logout behavior. If this page should instead rely on the shared
navbar, remove the duplicate hardcoded menu entirely.
Comment on lines
+155
to
+166
| <a href="index.html#dashboard" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-tachometer-alt"></i> Dashboard</a | ||
| > | ||
| <a href="resume.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-file-alt"></i> Coding Resume</a | ||
| > | ||
| <a href="/execution-history.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-history"></i> Execution History</a | ||
| > | ||
| <a href="/pages/career/resume/resume.html" class="dropdown-item" role="menuitem" | ||
| ><i class="fas fa-file-alt"></i> Coding Resume</a | ||
| > |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Duplicate "Coding Resume" links with different hrefs.
Line 158 links to resume.html (relative) and line 164 links to /pages/career/resume/resume.html (absolute), both labeled "Coding Resume" with the same icon. The relative path likely resolves incorrectly from /pages/Dsa-Battle/. Remove one — the absolute path (line 164) matches the navbar partial and is the correct one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/Dsa-Battle/dsa-battle-mode.html` around lines 155 - 166, The dropdown
menu in dsa-battle-mode.html has two “Coding Resume” entries pointing to
different hrefs, which creates a duplicate navigation item. Remove the incorrect
relative resume.html link from the menu and keep the absolute
/pages/career/resume/resume.html target so the dropdown matches the navbar
partial and resolves correctly from the Dsa-Battle page.
Comment on lines
+459
to
+469
| <script src="/socket.io/socket.io.js"></script> | ||
| <script src="dsa-battle-mode.js"></script> | ||
| <script src="/bootstrap-legacy.js"></script> | ||
|
|
||
| <script src="script.js"></script> | ||
| <script src="script.js"></script> | ||
| <script type="module" src="/dist/legacy-bundle.mjs"></script> | ||
| <script src="/theme.js"></script> | ||
| <script src="/script.js"></script> | ||
| <script src="/theme.js"></script> | ||
| <script src="/script.js"></script> | ||
| <script type="module" src="/dist/legacy-bundle.mjs"></script> | ||
| <script src="/auth.js"></script> | ||
| <script src="/theme.js"></script> | ||
| <script> | ||
| loadPartial('navbar-placeholder', '/partials/navbar.html'); | ||
| loadPartial('footer-placeholder', '/partials/footer.html'); | ||
| </script> | ||
| </body> | ||
| <script src="/auth.js"></script> | ||
| <script src="/theme.js"></script> |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔴 Critical | ⚡ Quick win
Duplicate script includes cause double-execution and wasted requests.
/dist/legacy-bundle.mjs is loaded at lines 464 and 467, and /theme.js at lines 465 and 469 — exact duplicates. script.js (relative, line 463) and /script.js (absolute, line 466) likely resolve to the same file. Double-loading ES modules and theme scripts can cause re-initialization errors, event listener duplication, and degraded Core Web Vitals (INP/LCP).
🔧 Proposed fix — remove duplicate script tags
<script src="/socket.io/socket.io.js"></script>
<script src="dsa-battle-mode.js"></script>
<script src="/bootstrap-legacy.js"></script>
<script src="script.js"></script>
<script type="module" src="/dist/legacy-bundle.mjs"></script>
<script src="/theme.js"></script>
-<script src="/script.js"></script>
-<script type="module" src="/dist/legacy-bundle.mjs"></script>
<script src="/auth.js"></script>
-<script src="/theme.js"></script>
<script>
loadPartial('navbar-placeholder', '/partials/navbar.html');
loadPartial('footer-placeholder', '/partials/footer.html');
</script>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <script src="/socket.io/socket.io.js"></script> | |
| <script src="dsa-battle-mode.js"></script> | |
| <script src="/bootstrap-legacy.js"></script> | |
| <script src="script.js"></script> | |
| <script src="script.js"></script> | |
| <script type="module" src="/dist/legacy-bundle.mjs"></script> | |
| <script src="/theme.js"></script> | |
| <script src="/script.js"></script> | |
| <script src="/theme.js"></script> | |
| <script src="/script.js"></script> | |
| <script type="module" src="/dist/legacy-bundle.mjs"></script> | |
| <script src="/auth.js"></script> | |
| <script src="/theme.js"></script> | |
| <script> | |
| loadPartial('navbar-placeholder', '/partials/navbar.html'); | |
| loadPartial('footer-placeholder', '/partials/footer.html'); | |
| </script> | |
| </body> | |
| <script src="/auth.js"></script> | |
| <script src="/theme.js"></script> | |
| <script src="/socket.io/socket.io.js"></script> | |
| <script src="dsa-battle-mode.js"></script> | |
| <script src="/bootstrap-legacy.js"></script> | |
| <script src="script.js"></script> | |
| <script type="module" src="/dist/legacy-bundle.mjs"></script> | |
| <script src="/theme.js"></script> | |
| <script src="/auth.js"></script> | |
| <script> | |
| loadPartial('navbar-placeholder', '/partials/navbar.html'); | |
| loadPartial('footer-placeholder', '/partials/footer.html'); | |
| </script> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/Dsa-Battle/dsa-battle-mode.html` around lines 459 - 469, The page
script block in dsa-battle-mode.html includes duplicate resources that can
execute twice and add unnecessary requests. Update the script tags around the
existing dsa-battle-mode.js, script.js, /script.js, /dist/legacy-bundle.mjs, and
/theme.js entries so each asset is included only once, keeping the intended
order while removing the redundant duplicate includes. Double-check the final
script list to ensure the module bundle and theme initialization are not loaded
more than once.
Source: Coding guidelines
Comment on lines
+207
to
+242
| .adt-option-btn { | ||
| background: rgba(255, 255, 255, 0.02); | ||
| border: 1px solid var(--glass-border); | ||
| border-radius: 12px; | ||
| padding: 1.25rem; | ||
| color: var(--text-primary); | ||
| font-size: 1rem; | ||
| font-weight: 500; | ||
| cursor: pointer; | ||
| transition: all 0.25s cubic-bezier(0.4, 0, 0.2, 1); | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| gap: 0.75rem; | ||
| } | ||
|
|
||
| .adt-option-btn i { | ||
| font-size: 1.5rem; | ||
| color: var(--text-secondary); | ||
| transition: color 0.25s ease; | ||
| } | ||
|
|
||
| .adt-option-btn:hover { | ||
| background: rgba(255, 255, 255, 0.07); | ||
| border-color: var(--accent); | ||
| box-shadow: 0 4px 20px rgba(6, 182, 212, 0.15); | ||
| transform: translateY(-3px); | ||
| } | ||
|
|
||
| .adt-option-btn:hover i { | ||
| color: var(--accent); | ||
| } | ||
|
|
||
| .adt-option-btn:active { | ||
| transform: translateY(-1px); | ||
| } |
Contributor
There was a problem hiding this comment.
Add :focus-visible styles for keyboard navigation.
All interactive elements (.adt-option-btn, .adt-tab-btn, .adt-copy-btn, .adt-filter-btn, .adt-library-card) have :hover and :active styles but no :focus-visible styles. Keyboard users have no visible focus indicator. As per coding guidelines, ensure WCAG 2.2 compliance and audit for accessibility issues.
♿ Proposed focus-visible styles
.adt-option-btn:active {
transform: translateY(-1px);
}
+
+.adt-option-btn:focus-visible,
+.adt-tab-btn:focus-visible,
+.adt-copy-btn:focus-visible,
+.adt-filter-btn:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}
+
+.adt-library-card:focus-visible {
+ outline: 2px solid var(--accent);
+ outline-offset: 2px;
+}Note: .adt-library-card and .adt-trail-step are <div> elements and also need tabindex="0" in the HTML/JS to be keyboard focusable.
Also applies to: 476-510, 661-684, 696-712
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.css` around lines
207 - 242, Add visible :focus-visible styles for the interactive controls in
algorithm-decision-tree.css, including .adt-option-btn, .adt-tab-btn,
.adt-copy-btn, .adt-filter-btn, .adt-library-card, and .adt-trail-step, so
keyboard users get a clear focus indicator matching the existing hover/active
treatment. Update the relevant style blocks for these selectors to include
accessible focus states, and make sure the non-button elements like
.adt-library-card and .adt-trail-step are keyboard focusable by adding
tabindex="0" in the corresponding HTML/JS. Keep the changes consistent with the
existing design and WCAG 2.2 expectations.
Source: Coding guidelines
Comment on lines
+151
to
+168
| <div class="adt-code-tabs"> | ||
| <div class="adt-tab-buttons"> | ||
| <button type="button" class="adt-tab-btn active" data-lang="js"> | ||
| <i class="fab fa-js"></i> JavaScript | ||
| </button> | ||
| <button type="button" class="adt-tab-btn" data-lang="py"> | ||
| <i class="fab fa-python"></i> Python | ||
| </button> | ||
| </div> | ||
| <button type="button" class="adt-copy-btn" id="adtCopyBtn" aria-label="Copy Code"> | ||
| <i class="far fa-copy"></i> Copy | ||
| </button> | ||
| </div> | ||
|
|
||
| <!-- Code Box --> | ||
| <div class="adt-code-box"> | ||
| <pre><code class="language-javascript" id="adtCodeContent"></code></pre> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Add ARIA tab pattern to code viewer tabs.
The tab buttons and code box lack the required ARIA tab pattern (role="tablist", role="tab", aria-selected, aria-controls, role="tabpanel"). Screen reader users cannot perceive or operate the JS/Python code toggle. As per coding guidelines, implement ARIA patterns and screen reader support in interactive components.
♿ Proposed ARIA additions
<!-- Code Viewer Tabs -->
<div class="adt-code-tabs">
- <div class="adt-tab-buttons">
- <button type="button" class="adt-tab-btn active" data-lang="js">
+ <div class="adt-tab-buttons" role="tablist" aria-label="Code language">
+ <button type="button" class="adt-tab-btn active" data-lang="js" role="tab" aria-selected="true" aria-controls="adtCodeContent" id="adtTabJs">
<i class="fab fa-js"></i> JavaScript
</button>
- <button type="button" class="adt-tab-btn" data-lang="py">
+ <button type="button" class="adt-tab-btn" data-lang="py" role="tab" aria-selected="false" aria-controls="adtCodeContent" id="adtTabPy">
<i class="fab fa-python"></i> Python
</button>
</div>And on the code box:
<!-- Code Box -->
- <div class="adt-code-box">
- <pre><code class="language-javascript" id="adtCodeContent"></code></pre>
+ <div class="adt-code-box" role="tabpanel" aria-labelledby="adtTabJs" tabindex="0">
+ <pre><code class="language-javascript" id="adtCodeContent"></code></pre>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div class="adt-code-tabs"> | |
| <div class="adt-tab-buttons"> | |
| <button type="button" class="adt-tab-btn active" data-lang="js"> | |
| <i class="fab fa-js"></i> JavaScript | |
| </button> | |
| <button type="button" class="adt-tab-btn" data-lang="py"> | |
| <i class="fab fa-python"></i> Python | |
| </button> | |
| </div> | |
| <button type="button" class="adt-copy-btn" id="adtCopyBtn" aria-label="Copy Code"> | |
| <i class="far fa-copy"></i> Copy | |
| </button> | |
| </div> | |
| <!-- Code Box --> | |
| <div class="adt-code-box"> | |
| <pre><code class="language-javascript" id="adtCodeContent"></code></pre> | |
| </div> | |
| <div class="adt-code-tabs"> | |
| <div class="adt-tab-buttons" role="tablist" aria-label="Code language"> | |
| <button type="button" class="adt-tab-btn active" data-lang="js" role="tab" aria-selected="true" aria-controls="adtCodeContent" id="adtTabJs"> | |
| <i class="fab fa-js"></i> JavaScript | |
| </button> | |
| <button type="button" class="adt-tab-btn" data-lang="py" role="tab" aria-selected="false" aria-controls="adtCodeContent" id="adtTabPy"> | |
| <i class="fab fa-python"></i> Python | |
| </button> | |
| </div> | |
| <button type="button" class="adt-copy-btn" id="adtCopyBtn" aria-label="Copy Code"> | |
| <i class="far fa-copy"></i> Copy | |
| </button> | |
| </div> | |
| <!-- Code Box --> | |
| <div class="adt-code-box" role="tabpanel" aria-labelledby="adtTabJs" tabindex="0"> | |
| <pre><code class="language-javascript" id="adtCodeContent"></code></pre> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.html` around
lines 151 - 168, The code viewer tabs in adt-code-tabs need the full ARIA tab
pattern so screen readers can identify and operate the JavaScript/Python toggle.
Update the tab controls in the code box UI to use role="tablist" on the
container, role="tab" with aria-selected and aria-controls on each button, and
role="tabpanel" on the code panel with the matching id and aria-labelledby
relationship; also keep the active state in sync in the tab switch logic for the
adt-tab-btn and adtCodeContent elements.
Source: Coding guidelines
Comment on lines
+206
to
+213
| <button type="button" class="adt-filter-btn active" data-filter="all"> | ||
| All Algorithms | ||
| </button> | ||
| <button type="button" class="adt-filter-btn" data-filter="search">Searching</button> | ||
| <button type="button" class="adt-filter-btn" data-filter="sort">Sorting</button> | ||
| <button type="button" class="adt-filter-btn" data-filter="graph">Graphs & Paths</button> | ||
| <button type="button" class="adt-filter-btn" data-filter="opt">DP & Arrays</button> | ||
| <button type="button" class="adt-filter-btn" data-filter="string">Strings</button> |
Contributor
There was a problem hiding this comment.
Add aria-pressed to library filter buttons.
Filter buttons function as toggle buttons but lack aria-pressed to convey their active/inactive state to assistive technology users. As per coding guidelines, implement ARIA patterns and screen reader support in interactive components.
♿ Proposed fix
- <button type="button" class="adt-filter-btn active" data-filter="all">
+ <button type="button" class="adt-filter-btn active" data-filter="all" aria-pressed="true">
All Algorithms
</button>
- <button type="button" class="adt-filter-btn" data-filter="search">Searching</button>
- <button type="button" class="adt-filter-btn" data-filter="sort">Sorting</button>
- <button type="button" class="adt-filter-btn" data-filter="graph">Graphs & Paths</button>
- <button type="button" class="adt-filter-btn" data-filter="opt">DP & Arrays</button>
- <button type="button" class="adt-filter-btn" data-filter="string">Strings</button>
+ <button type="button" class="adt-filter-btn" data-filter="search" aria-pressed="false">Searching</button>
+ <button type="button" class="adt-filter-btn" data-filter="sort" aria-pressed="false">Sorting</button>
+ <button type="button" class="adt-filter-btn" data-filter="graph" aria-pressed="false">Graphs & Paths</button>
+ <button type="button" class="adt-filter-btn" data-filter="opt" aria-pressed="false">DP & Arrays</button>
+ <button type="button" class="adt-filter-btn" data-filter="string" aria-pressed="false">Strings</button>The JS filterLibrary function should also toggle aria-pressed alongside the active class.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.html` around
lines 206 - 213, The library filter buttons in the algorithm decision tree act
like toggle buttons but only update the active class, so add aria-pressed state
handling in the filterLibrary function and keep it in sync with the current
filter selection. Update the button markup/state for the existing adt-filter-btn
controls so the active button has aria-pressed="true" and the inactive ones are
false, and ensure this toggles whenever the filter changes.
Source: Coding guidelines
Comment on lines
+1381
to
+1396
| let html = `<div class="adt-trail-step" data-index="-1">Home</div>`; | ||
|
|
||
| pathHistory.forEach((step, idx) => { | ||
| html += `<div class="adt-trail-arrow"><i class="fas fa-chevron-right"></i></div>`; | ||
| html += `<div class="adt-trail-step" data-index="${idx}">${step.choiceLabel}</div>`; | ||
| }); | ||
|
|
||
| trailStepsEl.innerHTML = html; | ||
|
|
||
| // Attach event listeners to breadcrumbs | ||
| trailStepsEl.querySelectorAll('.adt-trail-step').forEach((btn) => { | ||
| btn.addEventListener('click', () => { | ||
| const targetIndex = parseInt(btn.dataset.index); | ||
| rewindTo(targetIndex); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Breadcrumb steps are not keyboard accessible.
Breadcrumb steps are rendered as <div> elements with click handlers but no tabindex or role. Keyboard users cannot navigate or activate them. As per coding guidelines, ensure WCAG 2.2 compliance and implement ARIA patterns and screen reader support in interactive components.
♿ Proposed fix: use buttons instead of divs
- let html = `<div class="adt-trail-step" data-index="-1">Home</div>`;
+ let html = `<button type="button" class="adt-trail-step" data-index="-1">Home</button>`;
pathHistory.forEach((step, idx) => {
html += `<div class="adt-trail-arrow"><i class="fas fa-chevron-right"></i></div>`;
- html += `<div class="adt-trail-step" data-index="${idx}">${step.choiceLabel}</div>`;
+ html += `<button type="button" class="adt-trail-step" data-index="${idx}">${step.choiceLabel}</button>`;
});This makes them natively focusable and operable with Enter/Space.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let html = `<div class="adt-trail-step" data-index="-1">Home</div>`; | |
| pathHistory.forEach((step, idx) => { | |
| html += `<div class="adt-trail-arrow"><i class="fas fa-chevron-right"></i></div>`; | |
| html += `<div class="adt-trail-step" data-index="${idx}">${step.choiceLabel}</div>`; | |
| }); | |
| trailStepsEl.innerHTML = html; | |
| // Attach event listeners to breadcrumbs | |
| trailStepsEl.querySelectorAll('.adt-trail-step').forEach((btn) => { | |
| btn.addEventListener('click', () => { | |
| const targetIndex = parseInt(btn.dataset.index); | |
| rewindTo(targetIndex); | |
| }); | |
| }); | |
| let html = `<button type="button" class="adt-trail-step" data-index="-1">Home</button>`; | |
| pathHistory.forEach((step, idx) => { | |
| html += `<div class="adt-trail-arrow"><i class="fas fa-chevron-right"></i></div>`; | |
| html += `<button type="button" class="adt-trail-step" data-index="${idx}">${step.choiceLabel}</button>`; | |
| }); | |
| trailStepsEl.innerHTML = html; | |
| // Attach event listeners to breadcrumbs | |
| trailStepsEl.querySelectorAll('.adt-trail-step').forEach((btn) => { | |
| btn.addEventListener('click', () => { | |
| const targetIndex = parseInt(btn.dataset.index); | |
| rewindTo(targetIndex); | |
| }); | |
| }); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 1387-1387: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: trailStepsEl.innerHTML = html
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js` around lines
1381 - 1396, The breadcrumb items in the trail rendering are interactive but
currently built as plain divs, so they are not keyboard accessible. Update the
breadcrumb markup in the trail builder logic to use native buttons instead of
adt-trail-step divs, and keep the click behavior wired to rewindTo so Home and
each pathHistory step remain operable by keyboard and screen readers. Make sure
the unique breadcrumb rendering and listener code in the trailStepsEl/forEach
block preserves the same visual structure while adding proper accessible
semantics.
Source: Coding guidelines
Comment on lines
+1399
to
+1463
| function renderQuestion() { | ||
| const node = DECISION_TREE[currentNodeId]; | ||
| if (!node) return; | ||
|
|
||
| // Slide transition exit | ||
| questionPanel.classList.remove('adt-slide-enter'); | ||
| questionPanel.classList.add('adt-slide-exit'); | ||
|
|
||
| setTimeout(() => { | ||
| let optionsHtml = ''; | ||
| node.options.forEach((opt) => { | ||
| const iconClass = opt.icon || 'fa-circle-question'; | ||
| optionsHtml += ` | ||
| <button type="button" class="adt-option-btn" data-next="${opt.next}" data-label="${opt.label}"> | ||
| <i class="fas ${iconClass}"></i> | ||
| <span>${opt.label}</span> | ||
| </button> | ||
| `; | ||
| }); | ||
|
|
||
| // Question Icon | ||
| let qIcon = 'fa-network-wired'; | ||
| if (currentNodeId.includes('search')) qIcon = 'fa-magnifying-glass'; | ||
| else if (currentNodeId.includes('sort')) qIcon = 'fa-arrow-down-a-z'; | ||
| else if (currentNodeId.includes('graph')) qIcon = 'fa-diagram-project'; | ||
| else if (currentNodeId.includes('opt')) qIcon = 'fa-chart-line'; | ||
| else if (currentNodeId.includes('string')) qIcon = 'fa-font'; | ||
|
|
||
| questionPanel.innerHTML = ` | ||
| <div class="adt-question-icon"><i class="fas ${qIcon}"></i></div> | ||
| <h2 class="adt-question-text">${node.question}</h2> | ||
| <p class="adt-question-hint">${node.hint}</p> | ||
| <div class="adt-options-group">${optionsHtml}</div> | ||
| `; | ||
|
|
||
| // Back navigation button inside card if history is available | ||
| if (pathHistory.length > 0) { | ||
| questionPanel.innerHTML += ` | ||
| <div class="adt-nav-row"> | ||
| <button type="button" class="btn btn-secondary" id="adtCardBackBtn"> | ||
| <i class="fas fa-arrow-left"></i> Back | ||
| </button> | ||
| <span style="font-size: 0.8rem; color: var(--text-secondary)">Step ${pathHistory.length + 1}</span> | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| // Slide transition enter | ||
| questionPanel.classList.remove('adt-slide-exit'); | ||
| questionPanel.classList.add('adt-slide-enter'); | ||
|
|
||
| // Attach option listeners | ||
| questionPanel.querySelectorAll('.adt-option-btn').forEach((btn) => { | ||
| btn.addEventListener('click', () => { | ||
| handleChoice(btn.dataset.next, btn.dataset.label); | ||
| }); | ||
| }); | ||
|
|
||
| // Attach back listener | ||
| const backBtn = document.getElementById('adtCardBackBtn'); | ||
| if (backBtn) { | ||
| backBtn.addEventListener('click', goBackStep); | ||
| } | ||
| }, 200); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Race condition: option buttons remain clickable during slide transitions.
renderQuestion (200ms timeout) and showRecommendation (250ms timeout) do not disable the previous question's option buttons during the transition. A user can click multiple options before the new content renders, corrupting pathHistory with duplicate entries from the same node level.
🔒 Proposed fix: disable interaction during transitions
let isTransitioning = false;
function renderQuestion() {
const node = DECISION_TREE[currentNodeId];
if (!node) return;
+ isTransitioning = true;
// Slide transition exit
questionPanel.classList.remove('adt-slide-enter');
questionPanel.classList.add('adt-slide-exit');
setTimeout(() => {
+ isTransitioning = false;
// ... existing render logic ...
}, 200);
}
function handleChoice(nextNodeId, choiceLabel) {
+ if (isTransitioning) return;
// Record current step in history
pathHistory.push({Apply the same guard in showRecommendation and goBackStep.
Also applies to: 1526-1608
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 1406-1461: React's useState should not be directly called
Context: setTimeout(() => {
let optionsHtml = '';
node.options.forEach((opt) => {
const iconClass = opt.icon || 'fa-circle-question';
optionsHtml += <button type="button" class="adt-option-btn" data-next="${opt.next}" data-label="${opt.label}"> <i class="fas ${iconClass}"></i> <span>${opt.label}</span> </button>;
});
// Question Icon
let qIcon = 'fa-network-wired';
if (currentNodeId.includes('search')) qIcon = 'fa-magnifying-glass';
else if (currentNodeId.includes('sort')) qIcon = 'fa-arrow-down-a-z';
else if (currentNodeId.includes('graph')) qIcon = 'fa-diagram-project';
else if (currentNodeId.includes('opt')) qIcon = 'fa-chart-line';
else if (currentNodeId.includes('string')) qIcon = 'fa-font';
questionPanel.innerHTML = `
<div class="adt-question-icon"><i class="fas ${qIcon}"></i></div>
<h2 class="adt-question-text">${node.question}</h2>
<p class="adt-question-hint">${node.hint}</p>
<div class="adt-options-group">${optionsHtml}</div>
`;
// Back navigation button inside card if history is available
if (pathHistory.length > 0) {
questionPanel.innerHTML += `
<div class="adt-nav-row">
<button type="button" class="btn btn-secondary" id="adtCardBackBtn">
<i class="fas fa-arrow-left"></i> Back
</button>
<span style="font-size: 0.8rem; color: var(--text-secondary)">Step ${pathHistory.length + 1}</span>
</div>
`;
}
// Slide transition enter
questionPanel.classList.remove('adt-slide-exit');
questionPanel.classList.add('adt-slide-enter');
// Attach option listeners
questionPanel.querySelectorAll('.adt-option-btn').forEach((btn) => {
btn.addEventListener('click', () => {
handleChoice(btn.dataset.next, btn.dataset.label);
});
});
// Attach back listener
const backBtn = document.getElementById('adtCardBackBtn');
if (backBtn) {
backBtn.addEventListener('click', goBackStep);
}
}, 200)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(usestate-direct-usage)
[warning] 1426-1431: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: questionPanel.innerHTML = <div class="adt-question-icon"><i class="fas ${qIcon}"></i></div> <h2 class="adt-question-text">${node.question}</h2> <p class="adt-question-hint">${node.hint}</p> <div class="adt-options-group">${optionsHtml}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js` around lines
1399 - 1463, The transition logic in renderQuestion, showRecommendation, and
goBackStep allows repeated clicks before the new panel finishes rendering, which
can duplicate pathHistory entries. Add an interaction lock or disable the
current option/back controls as soon as a choice is made and keep them disabled
until the slide transition completes; then re-enable after the new content is
mounted. Use the existing renderQuestion, showRecommendation, and goBackStep
flow to place the guard consistently so clicks during the 200ms/250ms timeouts
are ignored.
Comment on lines
+1588
to
+1597
| if (algo.problems && algo.problems.length > 0) { | ||
| problemsList.innerHTML = algo.problems | ||
| .map( | ||
| (prob) => ` | ||
| <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link"> | ||
| <span><i class="fas fa-link"></i> ${prob.title}</span> | ||
| <span class="adt-problem-diff easy">Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span> | ||
| </a> | ||
| ` | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Difficulty badge is hardcoded as "easy" for all problems.
Every problem link renders with class="adt-problem-diff easy" and the text "Practice", regardless of actual problem difficulty. This is misleading to users. Consider adding a difficulty field to each problem entry in the ALGORITHMS object, or removing the difficulty badge if per-problem difficulty data isn't available.
✨ Proposed fix: remove misleading badge
- (prob) => `
- <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link">
- <span><i class="fas fa-link"></i> ${prob.title}</span>
- <span class="adt-problem-diff easy">Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span>
- </a>
- `
+ (prob) => `
+ <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link">
+ <span><i class="fas fa-link"></i> ${prob.title}</span>
+ <span>Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span>
+ </a>
+ `📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (algo.problems && algo.problems.length > 0) { | |
| problemsList.innerHTML = algo.problems | |
| .map( | |
| (prob) => ` | |
| <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link"> | |
| <span><i class="fas fa-link"></i> ${prob.title}</span> | |
| <span class="adt-problem-diff easy">Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span> | |
| </a> | |
| ` | |
| ) | |
| if (algo.problems && algo.problems.length > 0) { | |
| problemsList.innerHTML = algo.problems | |
| .map( | |
| (prob) => ` | |
| <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link"> | |
| <span><i class="fas fa-link"></i> ${prob.title}</span> | |
| <span>Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span> | |
| </a> | |
| ` | |
| ) |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 1588-1597: Avoid assigning untrusted data to innerHTML/outerHTML or document.write
Context: problemsList.innerHTML = algo.problems
.map(
(prob) => <a href="${prob.link}" target="_blank" rel="noopener noreferrer" class="adt-problem-link"> <span><i class="fas fa-link"></i> ${prob.title}</span> <span class="adt-problem-diff easy">Practice <i class="fas fa-arrow-up-right-from-square" style="font-size: 0.7rem;"></i></span> </a>
)
.join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting').
(inner-outer-html)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/tools/algorithm-decision-tree/algorithm-decision-tree.js` around lines
1588 - 1597, The problem link markup in the algorithm decision tree is
hardcoding the difficulty badge as “easy” and always showing “Practice,” which
is misleading. Update the `problemsList.innerHTML` rendering in
`algorithm-decision-tree.js` to either use a real per-problem difficulty value
from each `ALGORITHMS` entry (e.g. via `algo.problems` fields) or remove the
difficulty badge entirely if that data is not available. Make sure the change is
applied where the `prob.title`/`prob.link` anchor is generated so every problem
reflects accurate information.
Comment on lines
+446
to
+638
| function generateHtmlReport() { | ||
| if (!state.s) return; | ||
| const summary = getRoundsSummary(); | ||
| const dateStr = new Date().toLocaleString(); | ||
|
|
||
| let roundsHtml = ''; | ||
| summary.forEach((round) => { | ||
| let rowsHtml = ''; | ||
| round.ordering.forEach((suffixIndex, rowIdx) => { | ||
| const suffixSub = state.s.slice(suffixIndex); | ||
| const suffixSubDisplay = suffixSub.length > 20 ? suffixSub.slice(0, 20) + '…' : suffixSub; | ||
| const rank = round.ranks[suffixIndex] ?? 0; | ||
|
|
||
| let keyDisplay = '—'; | ||
| if (round.k > 0 || round.roundName !== 'Initialization') { | ||
| const r1 = round.ranks[suffixIndex]; | ||
| const r2Idx = suffixIndex + round.len; | ||
| const r2 = r2Idx < state.s.length ? round.ranks[r2Idx] : -1; | ||
| keyDisplay = `(${r1}, ${r2})`; | ||
| } | ||
|
|
||
| rowsHtml += ` | ||
| <tr> | ||
| <td>${rowIdx}</td> | ||
| <td>${suffixIndex}</td> | ||
| <td class="suffix">${suffixSubDisplay}</td> | ||
| <td>${rank}</td> | ||
| <td class="fira">${keyDisplay}</td> | ||
| </tr> | ||
| `; | ||
| }); | ||
|
|
||
| roundsHtml += ` | ||
| <div class="card"> | ||
| <h3>${round.roundName} (len = ${round.len})</h3> | ||
| <table> | ||
| <thead> | ||
| <tr> | ||
| <th>Row</th> | ||
| <th>Suffix Index</th> | ||
| <th>Suffix Substring</th> | ||
| <th>Rank</th> | ||
| <th>Key Pair</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| ${rowsHtml} | ||
| </tbody> | ||
| </table> | ||
| </div> | ||
| `; | ||
| }); | ||
|
|
||
| const finalOrderStr = state.finalOrder ? state.finalOrder.join(', ') : '—'; | ||
|
|
||
| const htmlContent = `<!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Suffix Array Visualization Report - ${state.s}</title> | ||
| <style> | ||
| body { | ||
| font-family: 'Poppins', -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; | ||
| background-color: #0a0a1a; | ||
| color: #e2e8f0; | ||
| line-height: 1.6; | ||
| padding: 2.5rem; | ||
| max-width: 900px; | ||
| margin: 0 auto; | ||
| } | ||
| header { | ||
| border-bottom: 2px solid #7c3aed; | ||
| padding-bottom: 1.5rem; | ||
| margin-bottom: 2rem; | ||
| } | ||
| h1 { | ||
| font-size: 2.5rem; | ||
| color: #06b6d4; | ||
| margin: 0 0 0.5rem 0; | ||
| text-transform: uppercase; | ||
| letter-spacing: 1px; | ||
| } | ||
| .metadata { | ||
| font-size: 0.9rem; | ||
| color: #a0aec0; | ||
| display: flex; | ||
| gap: 1.5rem; | ||
| flex-wrap: wrap; | ||
| margin-top: 1rem; | ||
| } | ||
| .metadata span strong { | ||
| color: #fff; | ||
| } | ||
| .card { | ||
| background: rgba(255, 255, 255, 0.03); | ||
| border: 1px solid rgba(255, 255, 255, 0.08); | ||
| border-radius: 12px; | ||
| padding: 1.5rem; | ||
| margin-bottom: 2rem; | ||
| box-shadow: 0 4px 20px rgba(0,0,0,0.3); | ||
| } | ||
| .card h3 { | ||
| margin-top: 0; | ||
| color: #a78bfa; | ||
| border-bottom: 1px solid rgba(255, 255, 255, 0.08); | ||
| padding-bottom: 0.5rem; | ||
| } | ||
| table { | ||
| width: 100%; | ||
| border-collapse: collapse; | ||
| margin-top: 1rem; | ||
| } | ||
| th, td { | ||
| padding: 0.75rem 1rem; | ||
| text-align: left; | ||
| border-bottom: 1px solid rgba(255, 255, 255, 0.06); | ||
| } | ||
| th { | ||
| color: #06b6d4; | ||
| font-weight: 600; | ||
| font-size: 0.9rem; | ||
| text-transform: uppercase; | ||
| } | ||
| td { | ||
| font-size: 0.95rem; | ||
| } | ||
| .suffix { | ||
| font-family: 'Fira Code', monospace; | ||
| color: #e2e8f0; | ||
| font-weight: 500; | ||
| } | ||
| .fira { | ||
| font-family: 'Fira Code', monospace; | ||
| } | ||
| .final-badge { | ||
| display: inline-block; | ||
| background: linear-gradient(135deg, #7c3aed 0%, #06b6d4 100%); | ||
| color: white; | ||
| padding: 0.75rem 1.25rem; | ||
| border-radius: 8px; | ||
| font-family: 'Fira Code', monospace; | ||
| font-size: 1.1rem; | ||
| font-weight: bold; | ||
| margin-top: 0.5rem; | ||
| box-shadow: 0 4px 15px rgba(6, 182, 212, 0.25); | ||
| } | ||
| footer { | ||
| text-align: center; | ||
| margin-top: 4rem; | ||
| font-size: 0.8rem; | ||
| color: #718096; | ||
| border-top: 1px solid rgba(255, 255, 255, 0.08); | ||
| padding-top: 1.5rem; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <header> | ||
| <h1>Suffix Array Report</h1> | ||
| <div class="metadata"> | ||
| <span>String: <strong>"${state.s}"</strong></span> | ||
| <span>Length: <strong>${state.s.length}</strong></span> | ||
| <span>Case-Insensitive: <strong>${state.caseInsensitive}</strong></span> | ||
| <span>Allow Spaces: <strong>${state.allowSpaces}</strong></span> | ||
| <span>Generated: <strong>${dateStr}</strong></span> | ||
| </div> | ||
| </header> | ||
|
|
||
| <div class="card"> | ||
| <h3>Final Suffix Array</h3> | ||
| <div class="final-badge">${finalOrderStr}</div> | ||
| </div> | ||
|
|
||
| <h2>Construction Rounds Summary</h2> | ||
| ${roundsHtml} | ||
|
|
||
| <footer> | ||
| Generated by Algo Infinity Verse Suffix Array Visualizer | ||
| </footer> | ||
| </body> | ||
| </html>`; | ||
|
|
||
| const blob = new Blob([htmlContent], { type: 'text/html' }); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement('a'); | ||
| a.href = url; | ||
| a.download = `suffix_array_${state.s.replace(/[^a-zA-Z0-9]/g, '_')}_report.html`; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| document.body.removeChild(a); | ||
| URL.revokeObjectURL(url); | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file ranges and search for the input flow / sinks mentioned.
FILE="pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js"
echo "== Outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== Lines around generateHtmlReport =="
sed -n '430,700p' "$FILE" | cat -n
echo
echo "== Search for normalizeInput, share-link, compareResult, innerHTML, textInput =="
rg -n "normalizeInput|compareResult|innerHTML|textInput|share|paramS|state\.s|generateHtmlReport" "$FILE"Repository: Eshajha19/Algo-Infinity-Verse
Length of output: 13858
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the source of user-controlled content and any escaping helpers.
FILE="pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js"
echo "== Input normalization and state assignment =="
sed -n '1,180p' "$FILE" | cat -n
echo
echo "== compareResult sink and report-related code =="
sed -n '800,900p' "$FILE" | cat -n
sed -n '1040,1105p' "$FILE" | cat -nRepository: Eshajha19/Algo-Infinity-Verse
Length of output: 14459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js"
echo "== Init and event wiring =="
sed -n '1110,1185p' "$FILE" | cat -n
echo
echo "== Existing escape helpers / HTML sinks =="
rg -n "escapeHtml|textContent|innerHTML|insertAdjacentHTML|document\.write|generateHtmlReport" "$FILE"Repository: Eshajha19/Algo-Infinity-Verse
Length of output: 6079
Escape user-controlled values before interpolating HTML. generateHtmlReport() writes state.s and suffix previews straight into the report <title>, metadata, and table cells, and computeAndDisplayComparison() also pushes raw previews into el.compareResult.innerHTML. A crafted input from the shared string flow can turn into script execution in both the downloaded report and the on-page comparison panel. Use one escaping helper for every user-derived value before any HTML interpolation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pages/visualizers/suffix-array-visualizer/suffix-array-visualizer.js` around
lines 446 - 638, The HTML rendering in generateHtmlReport and the comparison UI
in computeAndDisplayComparison are interpolating user-controlled string values
directly into HTML, which can lead to script injection. Add and use a single
escaping helper for every derived value from state.s, suffix previews, metadata,
and comparison output before inserting them into the report template or any
innerHTML assignment. Apply the fix consistently wherever generateHtmlReport
builds title/rows and where computeAndDisplayComparison writes to
el.compareResult.
Source: Coding guidelines
|
🎉 Thank you for opening this Pull Request! We appreciate your contribution and effort toward improving this project. A maintainer will review your PR soon. Please ensure: Thank you for contributing to the community. |
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.
closes #1936
Summary by CodeRabbit
New Features
UI Improvements