Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
63771f3
Resolve merge conflict with storybooks
TheNonPirate May 18, 2026
3788a4d
Resolve merge conflicts with storybooks
TheNonPirate May 18, 2026
a0cc732
Resolve merge conflicts with storybooks
TheNonPirate May 18, 2026
53b9aa5
Made images work for non-docx storybook files
TheNonPirate Jul 25, 2025
3dfd030
Swiping storybook image now switches pages.
TheNonPirate Jul 25, 2025
43d13b8
Implemented paragraph style milestones
TheNonPirate Jul 29, 2025
085cf59
Inline styles implemented in storybooks
TheNonPirate Jul 30, 2025
357b474
Resolve merge conflicts with storybooks
TheNonPirate May 18, 2026
fdd9891
Implemented multi-level lists
TheNonPirate Aug 1, 2025
144508f
Removed unnecessary console.log
TheNonPirate Aug 1, 2025
dde4725
Removed story from list of unsupported types
TheNonPirate Aug 1, 2025
17bd59c
Removed unnecessary console.logs
TheNonPirate Aug 4, 2025
18255ea
Resolve merge conflicts with storybooks
TheNonPirate May 18, 2026
f839626
Implemented list conversion tests
TheNonPirate Aug 5, 2025
3ae0802
Converted lists to standalone milestones
TheNonPirate Aug 5, 2025
4dc23d4
Resolve merge conflicts with storybooks
TheNonPirate May 18, 2026
1d922fe
Fixed bug with level 2 unordered lists
TheNonPirate Aug 6, 2025
b4e1b0f
Resolve merge conflicts with Playwright tests
TheNonPirate May 18, 2026
88c56ce
Improved passing of parameters to runTests
TheNonPirate Aug 8, 2025
bd6a2b9
Fixed lint errors
TheNonPirate May 18, 2026
b84d2fe
Fixed storybook illustrations
TheNonPirate May 18, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
test-results
*.swp
.DS_Store
node_modules
Expand Down
4 changes: 4 additions & 0 deletions config/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export type BookConfig = {
};
}[];
bookTabs?: BookTabsConfig;
pageIllustrations?: {
num: number;
filename: string;
}[];
};

export type BookCollectionConfig = {
Expand Down
61 changes: 48 additions & 13 deletions convert/convertBooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,34 @@ function replaceVideoTags(
): string {
return text.replace(/\\video (.*)/g, '\\zvideo-s |id="$1"\\*\\zvideo-e\\*');
}
function replacePStyleTags(text: string, _bcId: string, _bookId: string): string {
return text.replace(/\\(p_[^\s]+)/g, '\\m \\zstyle |id="$1"\\*');
}
function replaceCStyleTags(text: string, _bcId: string, _bookId: string): string {
return text.replace(/\\(c_[^\s]+)(.*?)\\\1\*/g, '\\zcstyle-s |id="$1"\\*$2\\zcstyle-e\\*');
}
/**
* Convert list tags to milestones
*/
export function transformLists(text: string, _bcId: string, _bookId: string): string {
text = transformZuliTags(text);
text = transformZoliTags(text);
text = transformZonTags(text);
return text;
}

// This is the start of supporting story books, but it still fails if there is no chapter.
function replacePageTags(
text: string,
_bcId: string,
_bookId: string,
_ctx: ConvertBookContext
): string {
return text.replace(/\\page (.*)/g, '\\zpage-s |id="$1"\\*\\zpage-e\\*');
function transformZuliTags(usfm: string): string {
return usfm.replace(/(\\zuli\d+)/g, '\\nb $1\\*');
}

function transformZoliTags(usfm: string): string {
return usfm.replace(/(\\zoli\d+)/g, '\\nb $1\\*');
}

function transformZonTags(usfm: string): string {
return usfm.replace(/(\\zon\d+)\s(\d+)/g, '$1 |start="$2"\\*');
}

function loadGlossary(collection: any, dataDir: string): string[] {
const glossary: string[] = [];
for (const book of collection.books) {
Expand Down Expand Up @@ -354,7 +372,9 @@ type FilterFunction = (
const usfmFilterFunctions: FilterFunction[] = [
removeStrongNumberReferences,
replaceVideoTags,
replacePageTags,
replacePStyleTags,
replaceCStyleTags,
transformLists,
convertMarkdownsToMilestones,
encodeJmpLinks,
handleNoCaptionFigures,
Expand All @@ -371,7 +391,8 @@ function applyFilters(
filterFunctions: FilterFunction[],
bcId: string,
bookId: string,
context: ConvertBookContext
context: ConvertBookContext,
bookType?: string
): string {
let filteredText = text;
for (const filterFn of filterFunctions) {
Expand Down Expand Up @@ -402,7 +423,7 @@ type ConvertBookContext = {
bcId: string;
};

const unsupportedBookTypes = ['story', 'songs', 'audio-only', 'bloom-player', 'quiz', 'undefined'];
const unsupportedBookTypes = ['songs', 'audio-only', 'bloom-player', 'quiz', 'undefined'];
export async function convertBooks(
dataDir: string,
scriptureConfig: ScriptureConfig,
Expand Down Expand Up @@ -481,7 +502,6 @@ export async function convertBooks(
for (const book of collection.books) {
let bookConverted = false;
switch (book.type) {
case 'story':
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that it will fall to the default case, but I think it would be better to include case 'story': above default:

case 'songs':
case 'audio-only':
case 'bloom-player':
Expand All @@ -502,6 +522,7 @@ export async function convertBooks(
});
displayBookId(context.bcId, book.id);
break;
case 'story':
default:
bookConverted = true;
if (book.format === 'html') {
Expand Down Expand Up @@ -856,8 +877,22 @@ function convertScriptureBook(
const filePath = path.join(context.dataDir, 'books', context.bcId, file);
fileContents.push(fs.readFileSync(filePath, 'utf-8'));
});
// Collect the file contents into a single document
let usfm: string;

if (book.type === 'story') {
// The first file contains meta-content (id, title, etc)
usfm = fileContents[0];

processBookContent(resolve, null, fileContents.join(''));
// Subsequent files represent storybook pages.
// SAB deletes the \page tags. Replace them with chapter tags.
for (let i = 1; i < fileContents.length; i++) {
usfm += `\\c ${i} ${fileContents[i]}`;
}
} else {
usfm = fileContents.join('');
}
processBookContent(resolve, null, usfm);
}
})
);
Expand Down
18 changes: 17 additions & 1 deletion convert/convertConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,7 @@ export function parseBookCollections(document: Document, dataDir: string, verbos
}
const audio: BookCollectionAudioConfig[] = [];
let chaptersLabels: { [key: string]: string } | undefined;
const pageIllustrations: { num: number; filename: string }[] = [];
for (const page of book.getElementsByTagName('page')) {
if (verbose >= 2) {
console.log(`.. page: ${page.attributes[0].value}`);
Expand All @@ -588,6 +589,20 @@ export function parseBookCollections(document: Document, dataDir: string, verbos
const chapterNum = page.attributes.getNamedItem('num')!.value;
chaptersLabels[chapterNum] = char;
}
const imageFileTag = page.getElementsByTagName('image-filename')[0];
if (imageFileTag) {
pageIllustrations.push({
num: Number(page.attributes.getNamedItem('num')?.value),
filename: book.getElementsByTagName('images')[0]
? tag.id +
'-' +
book.attributes.getNamedItem('id')!.value +
'-' +
imageFileTag.innerHTML
: imageFileTag.innerHTML
});
Comment on lines +592 to +603
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when page num is missing or invalid.

At Line 595, Number(page.attributes.getNamedItem('num')?.value) can silently yield NaN, which creates unusable pageIllustrations entries instead of surfacing bad XML immediately.

Suggested fix
                 const imageFileTag = page.getElementsByTagName('image-filename')[0];
                 if (imageFileTag) {
+                    const pageNum = parseInt(page.attributes.getNamedItem('num')!.value, 10);
+                    if (Number.isNaN(pageNum)) {
+                        throw new Error(`Invalid page num for book ${book.id}`);
+                    }
                     pageIllustrations.push({
-                        num: Number(page.attributes.getNamedItem('num')?.value),
+                        num: pageNum,
                         filename: book.getElementsByTagName('images')[0]
                             ? tag.id +
                               '-' +

Based on learnings: For conversion modules under convert/, adopt fail-fast behavior and surface invalid/missing XML attributes rather than silently recovering.

🤖 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 `@convert/convertConfig.ts` around lines 592 - 603, The code silently creates
pageIllustrations entries with NaN for num; instead, validate the page 'num'
attribute on page (page.attributes.getNamedItem('num')) before pushing: parse it
to an integer, check !isNaN and that it is a positive integer (or meets expected
constraints), and if invalid or missing throw an Error with a clear message
including identifying context (e.g., page.tagName/id or surrounding book id) so
conversion fails fast; update the block that constructs pageIllustrations (the
code referencing page, imageFileTag, book and
page.attributes.getNamedItem('num')) to perform this validation and throw rather
than pushing entries with NaN.

}

const audioTag = page.getElementsByTagName('audio')[0];
if (!audioTag) {
continue;
Expand Down Expand Up @@ -783,7 +798,8 @@ export function parseBookCollections(document: Document, dataDir: string, verbos
style,
styles,
footer,
bookTabs
bookTabs,
pageIllustrations
});
if (verbose >= 3) {
console.log(`.... book: `, JSON.stringify(books[0]));
Expand Down
14 changes: 14 additions & 0 deletions convert/runTests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { spawn } from 'node:child_process';

const args = process.argv.slice(2).join(' ');
console.log('playwright test ' + args);
spawn('playwright test ' + args, {
shell: true,
stdio: 'inherit'
Comment on lines +3 to +7
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, let's verify the file exists and check its content
cat -n convert/runTests.ts

Repository: sillsdev/appbuilder-pwa

Length of output: 519


🏁 Script executed:

# Let's check the Node.js spawn documentation and common patterns
# First, let's see if there are any other spawn calls in the repository that use best practices
rg "spawn\(" --type ts -A 3 | head -50

Repository: sillsdev/appbuilder-pwa

Length of output: 414


🏁 Script executed:

# Let's verify Node.js spawn API behavior with a quick test
# Check if there's any documentation or test files that show the correct pattern
fd -e md -e txt -e js -e ts | xargs grep -l "spawn.*args" | head -5

Repository: sillsdev/appbuilder-pwa

Length of output: 87


🏁 Script executed:

# Let me verify the exit code propagation issue mentioned in the internal scratchpad
# Check if the code properly handles exit codes from both spawn calls
grep -A 10 "spawn('vitest" convert/runTests.ts

Repository: sillsdev/appbuilder-pwa

Length of output: 155


Forward argv as an argument array instead of a shell string.

process.argv.slice(2) preserves argument boundaries correctly. Joining it into a single string and passing to spawn('playwright test ' + args, { shell: true, ... }) causes the shell to re-parse the string, breaking arguments with spaces or quotes. For example, -t "story book" will be split into separate tokens and fail to match reliably. Use spawn('playwright', ['test', ...process.argv.slice(2)], { shell: false }) instead.

🤖 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 `@convert/runTests.ts` around lines 3 - 7, The current code builds a single
shell string using args and calls spawn with shell:true which breaks
quoted/space-containing args; change the spawn call to forward arguments as an
array by using process.argv.slice(2) directly (e.g. spawn('playwright', ['test',
...process.argv.slice(2)], { shell: false, stdio: 'inherit' })) so argument
boundaries created by process.argv.slice(2) are preserved; update any references
to the earlier args variable accordingly and remove the shell:true usage.

}).on('close', (code) => {
console.log('vitest ' + args);
spawn('vitest ' + args, {
shell: true,
stdio: 'inherit'
});
});
8 changes: 5 additions & 3 deletions convert/tests/sab/convertConfigSAB.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,11 @@ if (programType === 'DAB') {
expect(bkk.chaptersN).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.id).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.name).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.section).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.testament).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.abbreviation).not.toSatisfy((r) => r === '' || r === undefined);
if (bkk.type !== 'story') {
expect(bkk.section).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.testament).not.toSatisfy((r) => r === '' || r === undefined);
expect(bkk.abbreviation).not.toSatisfy((r) => r === '' || r === undefined);
}
expect(bkk.file).not.toSatisfy((r) => r === '' || r === undefined);

for (const audio in bkk.audio) {
Expand Down
195 changes: 195 additions & 0 deletions convert/tests/sab/storybook.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
import { expect, test } from 'vitest';
import { transformLists } from '../../convertBooks';

function tokensOf(str: string) {
return str.split(/\s+/).filter((token) => token.length);
}

test('convert unordered list to milestones', () => {
const input = `
\\m Some content
\\zuli1 One
\\zuli1 Two \\zuli1 Three in a row!
\\b
`;
const expected = `
\\m Some content
\\nb \\zuli1\\* One
\\nb \\zuli1\\* Two
\\nb \\zuli1\\* Three in a row!
\\b
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert ordered list to milestones', () => {
const input = `
\\m Some content
\\zon1 10
\\zoli1 One
\\zoli1 Two \\zoli1 Three in a row!
\\b
`;
const expected = `
\\m Some content
\\zon1 |start="10"\\*
\\nb \\zoli1\\* One
\\nb \\zoli1\\* Two
\\nb \\zoli1\\* Three in a row!
\\b
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert multilevel unordered list to milestones', () => {
const input = `
\\c 2
\\b
\\zuli1 Old Testament
\\zuli2 Pentateuch
\\zuli3 Genesis
\\zuli3 Exodus
\\zuli3 Leviticus
\\zuli2 Joshua
\\zuli2 Judges
\\zuli1 New Testament
\\zuli2 Matthew
\\zuli2 Mark
\\zuli2 Luke
\\zuli2 John
\\zuli1 Glossary
`;
const expected = `
\\c 2
\\b
\\nb \\zuli1\\* Old Testament
\\nb \\zuli2\\* Pentateuch
\\nb \\zuli3\\* Genesis
\\nb \\zuli3\\* Exodus
\\nb \\zuli3\\* Leviticus
\\nb \\zuli2\\* Joshua
\\nb \\zuli2\\* Judges
\\nb \\zuli1\\* New Testament
\\nb \\zuli2\\* Matthew
\\nb \\zuli2\\* Mark
\\nb \\zuli2\\* Luke
\\nb \\zuli2\\* John
\\nb \\zuli1\\* Glossary
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert multilevel ordered list to milestones', () => {
const input = `
\\m My List:
\\b
\\zon1 1
\\zoli1 Food
\\zon2 1
\\zoli2 Fruit
\\zon3 1
\\zoli3 Apples
\\zoli3 Bananas
\\zoli3 Pears
\\zoli2 Dessert
\\zoli3 Pie
\\zoli3 Cake
\\zoli3 Ice Cream
\\zoli1 Drinks
\\zoli2 Coffee
\\zoli2 Water
\\zoli2 Tea
`;
const expected = `
\\m My List:
\\b
\\zon1 |start="1"\\*
\\nb \\zoli1\\* Food
\\zon2 |start="1"\\*
\\nb \\zoli2\\* Fruit
\\zon3 |start="1"\\*
\\nb \\zoli3\\* Apples
\\nb \\zoli3\\* Bananas
\\nb \\zoli3\\* Pears
\\nb \\zoli2\\* Dessert
\\nb \\zoli3\\* Pie
\\nb \\zoli3\\* Cake
\\nb \\zoli3\\* Ice Cream
\\nb \\zoli1\\* Drinks
\\nb \\zoli2\\* Coffee
\\nb \\zoli2\\* Water
\\nb \\zoli2\\* Tea
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert unordered list with formatting to milestones', () => {
const input = `
\\m Some content
\\zuli1 One
\\zuli1 \\bd Two \\bd*
\\zuli1 Three in a row!
\\b
`;
const expected = `
\\m Some content
\\nb \\zuli1\\* One
\\nb \\zuli1\\* \\bd Two \\bd*
\\nb \\zuli1\\* Three in a row!
\\b
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert ordered list with formatting to milestones', () => {
const input = `
\\m Some content
\\zon1 10
\\zoli1 One
\\zoli1 \\bdit Two \\bdit*
\\zoli1 Three in a row!
\\b
`;
const expected = `
\\m Some content
\\zon1 |start="10"\\*
\\nb \\zoli1\\* One
\\nb \\zoli1\\* \\bdit Two \\bdit*
\\nb \\zoli1\\* Three in a row!
\\b
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});

test('convert multilevel ordered list with formatting to milestones', () => {
const input = `
\\m Some content
\\zon1 10
\\zoli1 One
\\zon2 3
\\zoli2 \\it sub-point 1 \\it*
\\zoli2 sub-point 2
\\zoli1 \\bdit Two \\bdit*
\\zoli1 Three in a row!
\\b
`;
const expected = `
\\m Some content
\\zon1 |start="10"\\*
\\nb \\zoli1\\* One
\\zon2 |start="3"\\*
\\nb \\zoli2\\* \\it sub-point 1 \\it*
\\nb \\zoli2\\* sub-point 2
\\nb \\zoli1\\* \\bdit Two \\bdit*
\\nb \\zoli1\\* Three in a row!
\\b
`;
const result = transformLists(input, '', '');
expect(tokensOf(result)).toEqual(tokensOf(expected));
});
Loading
Loading