From 94e6b3f62e1b11ad40feabd84d87ebd330dac2db Mon Sep 17 00:00:00 2001 From: Artem Akymenko Date: Thu, 30 Jul 2026 14:57:10 +0000 Subject: [PATCH] refactor: move calculate_text_length to domain, add build_chapter_payload to app-layer - Create domain/text_utils.py with canonical calculate_text_length (strips chapter markers, voice markers, metadata tags) - Remove calculate_text_length from utils.py and subtitle_utils.py - Update all imports to use domain.text_utils directly - Create application/chapter_selection.py with build_chapter_payload() (orchestrates preselection + character count + safety net) - Replace manual chapter logic in form.py with build_chapter_payload() - 22 new tests for text_utils and chapter_selection (1550 total) --- abogen/application/chapter_selection.py | 62 ++++++++++++ abogen/book_parser.py | 3 +- abogen/domain/text_utils.py | 22 +++++ abogen/pyqt/book_handler.py | 6 +- abogen/pyqt/gui.py | 6 +- abogen/pyqt/queue_manager_gui.py | 2 +- abogen/subtitle_utils.py | 11 --- abogen/text_extractor.py | 3 +- abogen/utils.py | 13 --- abogen/webui/routes/utils/form.py | 37 +------ tests/test_application_chapter_selection.py | 102 ++++++++++++++++++++ tests/test_domain_text_utils.py | 44 +++++++++ tests/test_text_extractor.py | 2 +- 13 files changed, 243 insertions(+), 70 deletions(-) create mode 100644 abogen/application/chapter_selection.py create mode 100644 abogen/domain/text_utils.py create mode 100644 tests/test_application_chapter_selection.py create mode 100644 tests/test_domain_text_utils.py diff --git a/abogen/application/chapter_selection.py b/abogen/application/chapter_selection.py new file mode 100644 index 0000000..473236b --- /dev/null +++ b/abogen/application/chapter_selection.py @@ -0,0 +1,62 @@ +"""Chapter selection helpers for the application layer. + +Builds chapter payloads with smart defaults (preselection based on +supplement score) and character counts. Used by both WebUI and PyQt. +""" + +from __future__ import annotations + +from typing import Any, Dict, List + +from abogen.domain.chapter_classification import ( + ensure_at_least_one_chapter_enabled, + should_preselect_chapter, +) +from abogen.domain.text_utils import calculate_text_length + + +def build_chapter_payload( + chapters: List[Any], + source_name: str = "", +) -> List[Dict[str, Any]]: + """Build a chapter payload with preselection and character counts. + + Args: + chapters: List of chapter-like objects with ``title`` and ``text`` attributes. + source_name: Fallback title for the placeholder chapter when *chapters* is empty. + + Returns: + List of chapter dicts ready for ``PendingJob.chapters`` or ``ChapterChunkConfig``. + """ + total = len(chapters) + payload: List[Dict[str, Any]] = [] + + for index, chapter in enumerate(chapters): + title = getattr(chapter, "title", "") or "" + text = getattr(chapter, "text", "") or "" + enabled = should_preselect_chapter(title, text, index, total) + payload.append( + { + "id": f"{index:04d}", + "index": index, + "title": title, + "text": text, + "characters": calculate_text_length(text), + "enabled": enabled, + } + ) + + if not payload: + payload.append( + { + "id": "0000", + "index": 0, + "title": source_name, + "text": "", + "characters": 0, + "enabled": True, + } + ) + + ensure_at_least_one_chapter_enabled(payload) + return payload diff --git a/abogen/book_parser.py b/abogen/book_parser.py index d81e743..19ca04e 100644 --- a/abogen/book_parser.py +++ b/abogen/book_parser.py @@ -12,7 +12,8 @@ import fitz # PyMuPDF import markdown from abogen.utils import detect_encoding -from abogen.subtitle_utils import clean_text, calculate_text_length +from abogen.subtitle_utils import clean_text +from abogen.domain.text_utils import calculate_text_length # Pre-compile frequently used regex patterns _BRACKETED_NUMBERS_PATTERN = re.compile(r"\[\s*\d+\s*\]") diff --git a/abogen/domain/text_utils.py b/abogen/domain/text_utils.py new file mode 100644 index 0000000..fb8f931 --- /dev/null +++ b/abogen/domain/text_utils.py @@ -0,0 +1,22 @@ +"""Text utility functions for the domain layer.""" + +from __future__ import annotations + +import re + +# Pre-compiled patterns for calculate_text_length +_METADATA_TAG_PATTERN = re.compile(r"<]*>>") +_CHAPTER_MARKER_PATTERN = re.compile(r"<]*>>") +_VOICE_MARKER_PATTERN = re.compile(r"<]*>>") + + +def calculate_text_length(text: str) -> int: + """Calculate character count, ignoring internal markers and newlines. + + Strips chapter markers, voice markers, and metadata tags before counting. + """ + text = _CHAPTER_MARKER_PATTERN.sub("", text) + text = _VOICE_MARKER_PATTERN.sub("", text) + text = _METADATA_TAG_PATTERN.sub("", text) + text = text.replace("\n", "").strip() + return len(text) diff --git a/abogen/pyqt/book_handler.py b/abogen/pyqt/book_handler.py index f2ea8c7..a3efe3c 100644 --- a/abogen/pyqt/book_handler.py +++ b/abogen/pyqt/book_handler.py @@ -36,10 +36,8 @@ from abogen.domain.metadata_extraction import ( format_metadata_tags, ) -from abogen.subtitle_utils import ( - clean_text, - calculate_text_length, -) +from abogen.subtitle_utils import clean_text +from abogen.domain.text_utils import calculate_text_length import os import logging diff --git a/abogen/pyqt/gui.py b/abogen/pyqt/gui.py index fca8c0f..892d203 100644 --- a/abogen/pyqt/gui.py +++ b/abogen/pyqt/gui.py @@ -70,10 +70,8 @@ from abogen.utils import ( LoadPipelineThread, ) -from abogen.subtitle_utils import ( - clean_text, - calculate_text_length, -) +from abogen.subtitle_utils import clean_text +from abogen.domain.text_utils import calculate_text_length from abogen.pyqt.conversion import ConversionThread, VoicePreviewThread, PlayAudioThread, ChapterOptionsDialog, TimestampDetectionDialog from abogen.pyqt.book_handler import HandlerDialog diff --git a/abogen/pyqt/queue_manager_gui.py b/abogen/pyqt/queue_manager_gui.py index 748d6a3..9a3a9d1 100644 --- a/abogen/pyqt/queue_manager_gui.py +++ b/abogen/pyqt/queue_manager_gui.py @@ -523,7 +523,7 @@ class QueueManager(QDialog): return attrs def add_files_from_paths(self, file_paths): - from abogen.subtitle_utils import calculate_text_length + from abogen.domain.text_utils import calculate_text_length from PyQt6.QtWidgets import QMessageBox import os diff --git a/abogen/subtitle_utils.py b/abogen/subtitle_utils.py index 73a37cf..87e519b 100644 --- a/abogen/subtitle_utils.py +++ b/abogen/subtitle_utils.py @@ -34,17 +34,6 @@ def clean_subtitle_text(text): return text.strip() -def calculate_text_length(text): - # Use pre-compiled patterns for better performance - # Ignore chapter markers, voice markers, and metadata patterns in a single pass - text = _CHAPTER_MARKER_PATTERN.sub("", text) - text = _VOICE_MARKER_PATTERN.sub("", text) - text = _METADATA_TAG_PATTERN.sub("", text) - # Ignore newlines and leading/trailing spaces - text = text.replace("\n", "").strip() - # Calculate character count - char_count = len(text) - return char_count def clean_text(text, *args, **kwargs): diff --git a/abogen/text_extractor.py b/abogen/text_extractor.py index 066a9da..b47c349 100644 --- a/abogen/text_extractor.py +++ b/abogen/text_extractor.py @@ -16,7 +16,8 @@ import markdown # type: ignore[import] from bs4 import BeautifulSoup, NavigableString # type: ignore[import] from ebooklib import epub # type: ignore[import] -from .utils import calculate_text_length, clean_text, detect_encoding +from .utils import clean_text, detect_encoding +from .domain.text_utils import calculate_text_length logger = logging.getLogger(__name__) diff --git a/abogen/utils.py b/abogen/utils.py index df95457..b1e71c6 100644 --- a/abogen/utils.py +++ b/abogen/utils.py @@ -428,19 +428,6 @@ def save_config(config): pass -def calculate_text_length(text): - # Ignore chapter markers - text = re.sub(r"<>", "", text) - # Ignore metadata patterns - text = re.sub(r"<]*>>", "", text) - # Ignore newlines - text = text.replace("\n", "") - # Ignore leading/trailing spaces - text = text.strip() - # Calculate character count - char_count = len(text) - return char_count - def get_gpu_acceleration(enabled): try: diff --git a/abogen/webui/routes/utils/form.py b/abogen/webui/routes/utils/form.py index 8966c24..a615be7 100644 --- a/abogen/webui/routes/utils/form.py +++ b/abogen/webui/routes/utils/form.py @@ -6,11 +6,7 @@ from flask import request, render_template, jsonify from flask.typing import ResponseReturnValue from abogen.domain.enums import Language -from abogen.domain.chapter_classification import ( - supplement_score, - should_preselect_chapter, - ensure_at_least_one_chapter_enabled, -) +from abogen.application.chapter_selection import build_chapter_payload from abogen.webui.service import PendingJob, JobStatus from abogen.webui.routes.utils.service import get_service from abogen.tts_plugin.utils import is_plugin_registered @@ -40,7 +36,7 @@ from abogen.domain.voice_resolution import ( from abogen.webui.routes.utils.entity import sync_pronunciation_overrides from abogen.webui.routes.utils.epub import job_download_flags from abogen.webui.routes.utils.common import split_profile_spec, extract_checkbox -from abogen.utils import calculate_text_length +from abogen.domain.text_utils import calculate_text_length from abogen.voice_profiles import serialize_profiles, normalize_profile_entry from abogen.chunking import ChunkLevel, build_chunks_for_chapters from abogen.tts_plugin.utils import get_default_voice @@ -641,34 +637,7 @@ def build_pending_job_from_extraction( getattr(extraction, "combined_text", "") ) chapters_source = getattr(extraction, "chapters", []) or [] - total_chapter_count = len(chapters_source) - chapters_payload: List[Dict[str, Any]] = [] - for index, chapter in enumerate(chapters_source): - enabled = should_preselect_chapter(chapter.title, chapter.text, index, total_chapter_count) - chapters_payload.append( - { - "id": f"{index:04d}", - "index": index, - "title": chapter.title, - "text": chapter.text, - "characters": calculate_text_length(chapter.text), - "enabled": enabled, - } - ) - - if not chapters_payload: - chapters_payload.append( - { - "id": "0000", - "index": 0, - "title": original_name, - "text": "", - "characters": 0, - "enabled": True, - } - ) - - ensure_at_least_one_chapter_enabled(chapters_payload) + chapters_payload = build_chapter_payload(chapters_source, source_name=original_name) raw_language = str(form.get("language") or "a").strip() or "a" try: diff --git a/tests/test_application_chapter_selection.py b/tests/test_application_chapter_selection.py new file mode 100644 index 0000000..90a3e52 --- /dev/null +++ b/tests/test_application_chapter_selection.py @@ -0,0 +1,102 @@ +"""Tests for application/chapter_selection.py.""" + +import sys +import os +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +from dataclasses import dataclass +from abogen.application.chapter_selection import build_chapter_payload + + +@dataclass +class FakeChapter: + title: str + text: str + + +class TestBuildChapterPayload: + def test_empty_chapters(self): + result = build_chapter_payload([], source_name="book.txt") + assert len(result) == 1 + assert result[0]["id"] == "0000" + assert result[0]["title"] == "book.txt" + assert result[0]["text"] == "" + assert result[0]["characters"] == 0 + assert result[0]["enabled"] is True + + def test_single_chapter_always_enabled(self): + chapters = [FakeChapter("Chapter 1", "Once upon a time.")] + result = build_chapter_payload(chapters) + assert len(result) == 1 + assert result[0]["title"] == "Chapter 1" + assert result[0]["enabled"] is True + assert result[0]["index"] == 0 + assert result[0]["id"] == "0000" + + def test_content_chapters_preselected(self): + chapters = [ + FakeChapter("Chapter 1", "The story begins with a long enough text to pass the threshold."), + FakeChapter("Chapter 2", "The story continues with another substantial chunk of text."), + ] + result = build_chapter_payload(chapters) + assert all(ch["enabled"] for ch in result) + + def test_supplement_not_preselected(self): + chapters = [ + FakeChapter("Chapter 1", "The story begins with a long enough text to pass the threshold."), + FakeChapter("Title Page", ""), + FakeChapter("Copyright", "All rights reserved."), + FakeChapter("Table of Contents", ""), + FakeChapter("Chapter 2", "The story continues with another substantial chunk of text."), + ] + result = build_chapter_payload(chapters) + titles_enabled = {ch["title"]: ch["enabled"] for ch in result} + assert titles_enabled["Chapter 1"] is True + assert titles_enabled["Chapter 2"] is True + assert titles_enabled["Title Page"] is False + assert titles_enabled["Copyright"] is False + assert titles_enabled["Table of Contents"] is False + + def test_at_least_one_enabled(self): + chapters = [ + FakeChapter("Title Page", ""), + FakeChapter("Copyright", "All rights reserved."), + ] + result = build_chapter_payload(chapters) + assert any(ch["enabled"] for ch in result) + + def test_characters_calculated(self): + chapters = [FakeChapter("Ch1", "Hello world")] + result = build_chapter_payload(chapters) + assert result[0]["characters"] == 11 + + def test_ids_are_zero_padded(self): + chapters = [FakeChapter(f"Ch{i}", f"text {i}") for i in range(5)] + result = build_chapter_payload(chapters) + ids = [ch["id"] for ch in result] + assert ids == ["0000", "0001", "0002", "0003", "0004"] + + def test_indices_are_sequential(self): + chapters = [FakeChapter(f"Ch{i}", f"text {i}") for i in range(3)] + result = build_chapter_payload(chapters) + indices = [ch["index"] for ch in result] + assert indices == [0, 1, 2] + + def test_source_name_used_for_empty(self): + result = build_chapter_payload([], source_name="mybook.epub") + assert result[0]["title"] == "mybook.epub" + + def test_default_source_name(self): + result = build_chapter_payload([]) + assert result[0]["title"] == "" + + def test_none_title_and_text(self): + class BadChapter: + def __init__(self): + self.title = None + self.text = None + + result = build_chapter_payload([BadChapter()]) + assert result[0]["title"] == "" + assert result[0]["text"] == "" + assert result[0]["enabled"] is True # single chapter always enabled diff --git a/tests/test_domain_text_utils.py b/tests/test_domain_text_utils.py new file mode 100644 index 0000000..a0e596c --- /dev/null +++ b/tests/test_domain_text_utils.py @@ -0,0 +1,44 @@ +"""Tests for domain/text_utils.py.""" + +import sys +import os +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +from abogen.domain.text_utils import calculate_text_length + + +class TestCalculateTextUtilsLength: + def test_empty(self): + assert calculate_text_length("") == 0 + + def test_plain_text(self): + assert calculate_text_length("Hello world") == 11 + + def test_strips_newlines(self): + assert calculate_text_length("Hello\nworld") == 10 + + def test_strips_leading_trailing_spaces(self): + assert calculate_text_length(" Hello ") == 5 + + def test_strips_chapter_markers(self): + assert calculate_text_length("Hello<>world") == 10 + + def test_strips_voice_markers(self): + assert calculate_text_length("Hello<>world") == 10 + + def test_strips_metadata_tags(self): + assert calculate_text_length("Hello<>world") == 10 + + def test_strips_multiple_markers(self): + text = "<>Hello<> <>world" + assert calculate_text_length(text) == 11 + + def test_strips_mixed_content(self): + text = "<>\nHello\n<>\nworld\n" + assert calculate_text_length(text) == 10 + + def test_preserves_internal_spaces(self): + assert calculate_text_length("Hello world") == 11 + + def test_only_markers(self): + assert calculate_text_length("<><>") == 0 diff --git a/tests/test_text_extractor.py b/tests/test_text_extractor.py index fe41aff..986ea42 100644 --- a/tests/test_text_extractor.py +++ b/tests/test_text_extractor.py @@ -6,7 +6,7 @@ from pathlib import Path from ebooklib import epub from abogen.text_extractor import extract_from_path -from abogen.utils import calculate_text_length +from abogen.domain.text_utils import calculate_text_length @pytest.fixture