Conversation
- Align scroll view to top of new content blocks/lesson text when content overflows - Defer chapter completion on final step command output so full responses can be read/paged - Extract TypeScript utility modules for markdown, routing, and pager calculation - Add Vitest unit test suite for utilities and Vue components
…croll positioning - Align scroll view to top of new content blocks/lesson text when content overflows - Prevent image load DOM jumping using aspect ratios, CSS containment, and load event capturing - Defer chapter completion on final step command output so full responses can be read/paged - Extract TypeScript utility modules for markdown, routing, and pager calculation - Add Vitest unit test suite for utilities and Vue components
…tioning - Align scroll view to top of new content blocks/lesson text when content overflows - Prevent image load DOM jumping using aspect ratios, CSS containment, and load event capturing - Defer chapter completion on final step command output so full responses can be read/paged - Extract TypeScript utility modules for markdown, routing, and pager calculation - Add Vitest unit test suite for utilities and Vue components - Ensure fallback checks on error hint strings for steps without explicit ask targets
…tioning - Align scroll view to top of new content blocks/lesson text when content overflows - Prevent image load DOM jumping using aspect ratios, CSS containment, and load event capturing - Defer chapter completion on final step command output so full responses can be read/paged - Harden pager scroll calculation against sub-pixel display scale discrepancies - Extract TypeScript utility modules for markdown, routing, and pager calculation - Add Vitest unit test suite for utilities and Vue components
…tioning - Align scroll view to top of new content blocks/lesson text when content overflows - Prevent image load DOM jumping using aspect ratios, CSS containment, and load event capturing - Defer chapter completion on final step command output so full responses can be read/paged - Harden pager scroll calculation against sub-pixel display scale discrepancies - Extract TypeScript utility modules for markdown, routing, and pager calculation - Add Vitest unit test suite for utilities and Vue components
- Align scroll position to top of new blocks when content overflows - Defer chapter completion on final command responses until paged - Add :readonly and :inputmode="none" on command input during Pager mode - Refactor markdown, tutorial, and pager logic into TypeScript utilities - Add Vitest unit test suite covering utilities and Vue components
- Pin scroll to top on step 0 so chapter intro lesson text is read from the beginning - Activate pager mode when chapter intro text overflows terminal viewport height - Ensure smooth paging and mobile keyboard prevention across all tutorial chapters
…ove error feedback - Refactor TutorialPlayer.vue to an explicit Player State Machine - Ensure story beats look ahead to following quest prompts and advance cleanly after overflow - Improve unrecognized command error text to clarify tutorial step context and offer hints - Add unit tests covering state machine transitions and error feedback
…nsitions - Normalize the Chapter Complete prompt bar with gold completion styling and a glowing 'Next Chapter →' action button - Prevent virtual keyboard popups on mobile touch devices during Chapter Complete state - Refactor player state machine transitions across multi-step chapters - Add Vitest unit tests verifying completion bar prompt state
Refactor TutorialPlayer.vue to handle large step outputs and large initial lesson content without skipping content or auto-advancing premature chapter completions. - Implement state machine in TutorialPlayer.vue (AWAITING_COMMAND, PAGING_OUTPUT, PLAYING_BEAT, CHAPTER_COMPLETE) - Add top scroll alignment for long content blocks via scrollLogToLatestBlock() - Add touch-aware prompt bar copy for paging and chapter completion states - Extract markdown, tutorial, and pager utility functions into modular TS files - Add Vitest unit tests for utilities and components
… bar action pill UX - Add Player State Machine and deferred completion logic in TutorialPlayer.vue - Align viewport scroll position to the top of newly added content blocks - Replaced prompt input bar during output overflow with a device-tailored full-width action pill - Add unit test suite for markdown, tutorial, and pager utility modules
… action pill UX - Fix terminal output pagination and defer chapter completion state machine transitions - Align scroll position to top of new content blocks for large lesson text and long MUD command outputs - Replace prompt bar during output overflow and chapter completion with device-tailored full-width action pills - Clean up duplicate buttons in log completion cards and add unit tests for utility modules
…ling - Refactor terminal output paging logic into explicit PlayerState machine - Align viewport scrolling to top of newly added blocks instead of auto-scrolling to bottom - Defer chapter completion triggers when output overflows until paged through - Transform prompt input bar into full-width action pills for Paging and Chapter Complete states - Modularize markdown formatting, tutorial route normalization, and pager state utilities - Add comprehensive Vitest unit tests for tutorial player and utility modules
Contributor
Reviewer's GuideThe PR converts the tutorial player to TypeScript, reorganizes it around an explicit state machine for paging and chapter progression, improves Markdown/media and responsive controls, and adds Vitest-based utility, component, and player tests. State diagram for the tutorial player progressionstateDiagram-v2
[*] --> AWAITING_COMMAND
AWAITING_COMMAND --> PAGING_OUTPUT: isOverflowActive
PAGING_OUTPUT --> PAGING_OUTPUT: pageForward
PAGING_OUTPUT --> PLAYING_BEAT: output fully scrolled
PLAYING_BEAT --> AWAITING_COMMAND: advanceSubStep
AWAITING_COMMAND --> AWAITING_COMMAND: submit valid command
AWAITING_COMMAND --> CHAPTER_COMPLETE: final step completed
PAGING_OUTPUT --> CHAPTER_COMPLETE: final output fully scrolled
CHAPTER_COMPLETE --> AWAITING_COMMAND: navigateToUrl
Flow diagram for the Vitest tutorial test setupflowchart LR
TestFiles["Utility, component, and player tests"] --> Vitest["Vitest"]
Vitest --> VuePlugin["@vitejs/plugin-vue"]
VuePlugin --> JSDOM["jsdom test environment"]
JSDOM --> TestResults["Test results"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/.vitepress/theme/utils/markdown.ts" line_range="46-53" />
<code_context>
+ })
+
+ // Links: [text](url)
+ safe = safe.replace(/\[([^\]]+)\]\(([^)]+)\)/g, (_match, linkText, url) => {
+ const cleanText = linkText
+ let cleanUrl = url.trim()
+ if (!/^(?:https?:\/\/|mailto:)/i.test(cleanUrl) && cleanUrl.startsWith('/') && typeof withBaseFn === 'function') {
+ cleanUrl = withBaseFn(cleanUrl)
+ }
+ const isExternal = /^(?:https?:\/\/)/i.test(cleanUrl)
+ return `<a href="${cleanUrl}" ${isExternal ? 'target="_blank" rel="noopener noreferrer"' : ''} class="tut-link">${cleanText}</a>`
+ })
+
</code_context>
<issue_to_address>
**🚨 issue (security):** Markdown links are emitted into `v-html` without validating their scheme or escaping their URL attribute. A link such as `[x](javascript:...)` produces an executable `href`, and a URL containing a quote can break out of the attribute and inject markup.
**Triggers:** When tutorial markdown contains an untrusted or compromised link URL.
**Suggested fix:** Allow only explicitly safe schemes such as `https:`, `mailto:`, and relative URLs, and HTML-escape the final URL before inserting it into the attribute.
</issue_to_address>
### Comment 2
<location path="docs/.vitepress/theme/utils/tutorial.ts" line_range="29-32" />
<code_context>
+ */
+export function normalizeChapterPath(rawPath: string | undefined, basePrefix: string = '/'): string {
+ if (!rawPath) return '/'
+ let currentPath = rawPath.replace(/\.html$/, '').replace(/\/$/, '')
+ const base = basePrefix || '/'
+ if (base !== '/' && currentPath.startsWith(base.replace(/\/$/, ''))) {
+ currentPath = '/' + currentPath.slice(base.replace(/\/$/, '').length).replace(/^\//, '')
+ }
+ return currentPath || '/'
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** The base-prefix check matches any path that merely starts with the base text, not only the base path segment. For example, base `/pr-123/` also matches `/pr-1234/...`, strips the wrong prefix, and causes chapter lookup to select the fallback chapter.
**Triggers:** When a route path shares the base string but not its path-segment boundary.
**Suggested fix:** Require the base prefix to end at a path boundary, or normalize and compare the base with a trailing slash before stripping it.
```suggestion
const base = basePrefix || '/'
const basePath = base.replace(/\/$/, '')
if (base !== '/' && (currentPath === basePath || currentPath.startsWith(basePath + '/'))) {
currentPath = '/' + currentPath.slice(basePath.length).replace(/^\//, '')
}
```
</issue_to_address>
### Comment 3
<location path="docs/.vitepress/theme/components/TutorialPlayer.vue" line_range="237-240" />
<code_context>
}
+function handleMediaLoad() {
+ updatePagerState()
+}
+
function handleScroll() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Pager state is sampled once 300 ms after `scrollBy`, but `handleScroll` does not recompute it when the smooth scroll actually finishes. If the smooth animation lasts longer than the fixed timeout, the container remains marked as overflowing at the bottom and a pending chapter/beat transition is not executed until the user performs another paging action.
**Triggers:** When smooth scrolling takes longer than 300 ms, such as on a busy device or with a long scroll distance.
**Suggested fix:** Recompute pager state from the scroll-end/debounced scroll handler, or poll until scrolling has settled before executing the pending transition.
</issue_to_address>
### Comment 4
<location path="tests/components.test.ts" line_range="92-95" />
<code_context>
+ it('filters commands by category tab', async () => {
+ const wrapper = mount(MumeCommandGuide)
+ expect(wrapper.find('.command-table').exists()).toBe(true)
+ const filterTabs = wrapper.findAll('.filter-tab')
+ const gearTab = filterTabs.find(b => b.text().includes('Gear & Inventory'))
+ if (gearTab) {
+ await gearTab.trigger('click')
+ expect(wrapper.text()).toContain('equipment')
+ }
+ })
+})
</code_context>
<issue_to_address>
**issue (testing):** The test only performs its assertion inside `if (gearTab)`, so it passes without checking anything when the expected filter tab is absent. The test therefore cannot detect a regression that removes or renames the Gear & Inventory tab.
**Triggers:** When the component no longer renders the expected filter tab.
**Suggested fix:** Assert that `gearTab` exists before triggering it, then assert the filtered content.
```suggestion
expect(gearTab).toBeDefined()
await gearTab!.trigger('click')
expect(wrapper.text()).toContain('equipment')
```
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: docs/.vitepress/theme/utils/markdown.ts:53, docs/.vitepress/theme/utils/tutorial.ts:32, docs/.vitepress/theme/components/TutorialPlayer.vue:240, tests/components.test.ts:95
…tection - Validate link schemes (http, https, mailto, relative, anchors) and escape URL attributes in markdown utility - Enforce strict path-segment boundary checking in normalizeChapterPath - Add scrollend listener and debounced scroll settling for pager state updates - Strengthen unit test assertions in components test suite
…ete states - Add Enter keydown handling during CHAPTER_COMPLETE state to advance chapters or open end-of-tutorial modal on Chapter 15 - Add Enter keydown listener to pagination state to scroll terminal output - Update unit tests to verify keydown handling for chapter complete state
This branch was successfully deployed
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
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.
Summary by Sourcery
Improve the interactive tutorial player with typed state-driven progression, more reliable paging and navigation, richer Markdown rendering, and comprehensive automated tests.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: