diff --git a/gui/app.py b/gui/app.py index 2d56894..8cf6e04 100644 --- a/gui/app.py +++ b/gui/app.py @@ -1267,6 +1267,7 @@ def _style() -> None: } header[data-testid="stHeader"] { background: transparent !important; } [data-testid="stToolbar"], [data-testid="stDecoration"], #MainMenu, footer { display: none !important; } + [data-testid="stAppDeployButton"], [data-testid="stAppDeployButton"] button { display: none !important; } .block-container { padding: 1.25rem 3rem 3rem; max-width: 1420px; position: relative; z-index: 1; } .stApp h1, .stApp h2, .stApp h3, .stApp h4, .stApp [data-testid="stMarkdownContainer"] h1, @@ -1505,3 +1506,7 @@ def _style() -> None: unsafe_allow_html=True, ) + +if __name__ == "__main__": + main() + diff --git a/gui/studio_core.py b/gui/studio_core.py index 744c469..fb90bf0 100644 --- a/gui/studio_core.py +++ b/gui/studio_core.py @@ -196,9 +196,17 @@ def build_textbook_command(config: TextbookConfig) -> list[str]: def command_for_display(cmd: list[str]) -> str: - # Keys never travel through the command line (they are injected via env), - # so no redaction is needed here. - return " ".join(cmd) + redacted: list[str] = [] + redact_next = False + secret_flags = {"--api-key", "--vision-api-key", "--ocr-api-key", "--ocr-secret-key"} + for token in cmd: + if redact_next: + redacted.append("***") + redact_next = False + continue + redacted.append(token) + redact_next = token in secret_flags + return " ".join(redacted) def performance_tips(config: StudioConfig) -> list[str]: diff --git a/scripts/audit_redundancy.py b/scripts/audit_redundancy.py index 3fd6e25..763d912 100644 --- a/scripts/audit_redundancy.py +++ b/scripts/audit_redundancy.py @@ -12,6 +12,7 @@ from __future__ import annotations import ast +import importlib.util import sys from collections import defaultdict from pathlib import Path @@ -27,12 +28,17 @@ def iter_py_files(root: Path) -> list[Path]: - return sorted(p for p in root.rglob("*.py") if "__pycache__" not in p.parts) + ignored_parts = {".git", ".venv", "__pycache__", "gui_runs", "outputs"} + return sorted(p for p in root.rglob("*.py") if not ignored_parts.intersection(p.parts)) -def module_name(path: Path, root: Path) -> str: +def module_name(path: Path, root: Path, package_prefix: str | None = None) -> str: rel = path.relative_to(root) parts = list(rel.with_suffix("").parts) + if parts and parts[-1] == "__init__": + parts.pop() + if package_prefix: + parts.insert(0, package_prefix) return ".".join(parts) @@ -61,6 +67,8 @@ def analyze_file(path: Path) -> tuple[set[str], list[tuple[int, str]], list[ast. local = alias.asname or alias.name.split(".")[0] imported.append((node.lineno, local, alias.name)) elif isinstance(node, ast.ImportFrom): + if node.module == "__future__": + continue for alias in node.names: if alias.name == "*": continue @@ -81,31 +89,47 @@ def analyze_file(path: Path) -> tuple[set[str], list[tuple[int, str]], list[ast. def main() -> int: files = iter_py_files(PACKAGE) + repository_files = iter_py_files(ROOT) all_unused: list[tuple[str, int, str, str]] = [] body_hashes: dict[str, list[tuple[str, str]]] = defaultdict(list) # (importer_module, imported_module) edges; relative imports resolved. import_edges: set[tuple[str, str]] = set() - module_of: dict[Path, str] = {p: module_name(p, PACKAGE) for p in files} + module_of: dict[Path, str] = {p: module_name(p, PACKAGE, PACKAGE.name) for p in files} + + def importer_name(path: Path) -> str: + if path in module_of: + return module_of[path] + return module_name(path, ROOT) - def resolve_relative(base: str, level: int, module: str | None) -> str | None: - parts = base.split(".") - if level > len(parts): + def resolve_relative(importer: str, path: Path, level: int, module: str | None) -> str | None: + package = importer if path.name == "__init__.py" else importer.rpartition(".")[0] + if not package: + return None + try: + return importlib.util.resolve_name("." * level + (module or ""), package) + except (ImportError, ValueError): return None - prefix = parts[: len(parts) - level + 1] - if module: - return ".".join(prefix + module.split(".")) - return ".".join(prefix) - for path in files: - mod = module_of[path] + for path in repository_files: + mod = importer_name(path) tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) - _, unused, functions = analyze_file(path) - for lineno, name, src in unused: - all_unused.append((mod, lineno, name, src)) - for fn in functions: - wrapper = ast.Module(body=fn.body, type_ignores=[]) - h = ast.dump(wrapper, include_attributes=False) - body_hashes[h].append((mod, fn.name)) + if path in module_of: + _, unused, functions = analyze_file(path) + for lineno, name, src in unused: + all_unused.append((mod, lineno, name, src)) + for fn in functions: + if len(fn.body) == 1 and ( + isinstance(fn.body[0], ast.Pass) + or ( + isinstance(fn.body[0], ast.Expr) + and isinstance(fn.body[0].value, ast.Constant) + and fn.body[0].value.value is Ellipsis + ) + ): + continue + wrapper = ast.Module(body=fn.body, type_ignores=[]) + h = ast.dump(wrapper, include_attributes=False) + body_hashes[h].append((mod, fn.name)) for node in ast.walk(tree): if isinstance(node, ast.Import): for alias in node.names: @@ -114,15 +138,14 @@ def resolve_relative(base: str, level: int, module: str | None) -> str | None: if node.level == 0 and node.module: import_edges.add((mod, node.module)) elif node.level > 0: - resolved = resolve_relative(mod, node.level, node.module) + resolved = resolve_relative(mod, path, node.level, node.module) if resolved: import_edges.add((mod, resolved)) # A module is reachable if some other module imports it (or a submodule of it). reachable: set[str] = set() for importer, imported in import_edges: - if importer in module_of.values(): - reachable.add(imported) + reachable.add(imported) def is_imported(target: str) -> bool: return any(imp == target or imp.startswith(target + ".") or target.startswith(imp + ".") for imp in reachable) @@ -157,6 +180,12 @@ def is_imported(target: str) -> bool: for j in range(i + 1, len(locs)): m1, f1 = locs[i] m2, f2 = locs[j] + # Same-named methods in one module commonly implement one + # protocol contract; without qualified class names this pair + # is not actionable duplicate-helper evidence. Keep reporting + # same-named helpers duplicated across different modules. + if f1 == f2 and m1 == m2: + continue key = tuple(sorted([(m1, f1), (m2, f2)])) if key in seen_pairs: continue diff --git a/slidenote/extractors/pdf.py b/slidenote/extractors/pdf.py index ec965ca..90fb3a6 100644 --- a/slidenote/extractors/pdf.py +++ b/slidenote/extractors/pdf.py @@ -2,6 +2,7 @@ import contextlib import io +import threading from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path @@ -10,6 +11,9 @@ from slidenote.utils import unique_path +_TABLE_EXTRACTION_LOCK = threading.Lock() + + def extract_pdf(input_path: Path, output_root: Path, concurrency: int = 4) -> Deck: try: import fitz @@ -127,8 +131,12 @@ def _extract_tables(page: object, page_index: int) -> list[TableBlock]: if finder is None: return tables try: - with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): - found = finder() + # redirect_stdout/redirect_stderr mutate process-global streams. Page + # extraction is parallel, so serialize this small noisy call to prevent + # interleaved context managers from restoring the wrong stream object. + with _TABLE_EXTRACTION_LOCK: + with contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): + found = finder() except Exception: return tables for table_index, table in enumerate(getattr(found, "tables", []) or [], start=1): diff --git a/slidenote/figure_grounding.py b/slidenote/figure_grounding.py index a378dc0..40293cf 100644 --- a/slidenote/figure_grounding.py +++ b/slidenote/figure_grounding.py @@ -14,10 +14,12 @@ from slidenote.table_understanding import table_text_for_prompt from slidenote.utils import ( as_float, + clamp_normalized_bbox as _clamp_bbox, cleanup_temp_image, display_path, file_sha256, looks_normalized, + layout_order_from_bbox as _order_from_bbox, parse_json_object, prepare_image_for_api, preview, @@ -858,21 +860,6 @@ def _normalize_bbox(source_type: str, bbox: list[float] | None, page: SlidePage) return _clamp_bbox([x1 / width, y1 / height, x2 / width, y2 / height]) -def _clamp_bbox(bbox: list[float]) -> list[float]: - x1, y1, x2, y2 = [max(0.0, min(1.0, float(value))) for value in bbox] - if x2 < x1: - x1, x2 = x2, x1 - if y2 < y1: - y1, y2 = y2, y1 - return [round(x1, 4), round(y1, 4), round(x2, 4), round(y2, 4)] - - -def _order_from_bbox(bbox: list[float] | None) -> float: - if not bbox: - return 9999.0 - return round(float(bbox[1]) * 1000.0 + float(bbox[0]), 4) - - def _horizontal_overlap_ratio(a: list[float], b: list[float]) -> float: left = max(a[0], b[0]) right = min(a[2], b[2]) diff --git a/slidenote/notes/direct.py b/slidenote/notes/direct.py index e53b2e3..0cc7518 100644 --- a/slidenote/notes/direct.py +++ b/slidenote/notes/direct.py @@ -1,8 +1,9 @@ from __future__ import annotations from concurrent.futures import ThreadPoolExecutor, as_completed +from dataclasses import replace from pathlib import Path -from typing import Any, Callable +from typing import Any from slidenote.content_guard import record_repair from slidenote.llm import resolve_provider_runtime @@ -40,22 +41,26 @@ def _generate_notes_with_llm( resolved_base_url = runtime["base_url"] supports_image_input = bool(runtime["supports_image_input"]) resolved_cache_dir = (options.cache_dir or (output_root / ".cache" / "llm")).resolve() - cache = LLMCache(resolved_cache_dir, mode=options.cache_mode) - if options.note_strategy == "lecture-weave": + runtime_options = replace( + options, + provider=resolved_provider, + model=resolved_model, + base_url=resolved_base_url, + cache_dir=resolved_cache_dir, + ) + cache = LLMCache(resolved_cache_dir, mode=runtime_options.cache_mode) + if runtime_options.note_strategy == "lecture-weave": return _generate_notes_with_lecture_weave( deck=deck, output_root=output_root, - options=options, + options=runtime_options, note_depth=note_depth, asset_map=asset_map, - resolved_provider=resolved_provider, - resolved_model=resolved_model, - resolved_base_url=resolved_base_url, - resolved_cache_dir=resolved_cache_dir, cache=cache, supports_image_input=supports_image_input, ) + options = runtime_options contexts = _select_note_contexts(deck, options.note_context, section_plan=options.section_plan) resolved_note_context = _resolved_context_mode(deck, options.note_context) refresh_ids = options.refresh_slide_ids or set() diff --git a/slidenote/notes/lecture_weave.py b/slidenote/notes/lecture_weave.py index b35c816..be1c89f 100644 --- a/slidenote/notes/lecture_weave.py +++ b/slidenote/notes/lecture_weave.py @@ -2,7 +2,7 @@ from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path -from typing import Any, Callable +from typing import Any from slidenote.content_guard import record_repair from slidenote.llm_cache import LLMCache @@ -35,24 +35,22 @@ def _generate_notes_with_lecture_weave( *, note_depth: str, asset_map: dict[str, str], - resolved_provider: str, - resolved_model: str, - resolved_base_url: str, - resolved_cache_dir: Path, cache: LLMCache, supports_image_input: bool, ) -> "NoteGenerationResult": # string annotation to avoid circular import from . import NoteGenerationResult - # Unpack the option object into the locals the rest of this function uses. - provider = resolved_provider - model = resolved_model + # ``direct`` resolves provider runtime defaults once and stores them back + # into the immutable copy passed here, keeping calls, cache keys and reports + # on the same canonical provider/model/base URL/cache directory. + provider = options.provider + model = options.model api_key = options.api_key - base_url = resolved_base_url + base_url = options.base_url max_output_tokens = options.max_output_tokens temperature = options.temperature cache_mode = options.cache_mode - cache_dir = resolved_cache_dir + cache_dir = options.cache_dir concurrency = options.concurrency refresh_slide_ids = options.refresh_slide_ids progress_callback = options.progress_callback diff --git a/slidenote/semantic_layout.py b/slidenote/semantic_layout.py index 9f027cf..85b30e2 100644 --- a/slidenote/semantic_layout.py +++ b/slidenote/semantic_layout.py @@ -13,9 +13,11 @@ from slidenote.table_understanding import table_preview from slidenote.utils import ( as_float, + clamp_normalized_bbox as _clamp_bbox, cleanup_temp_image, display_path, file_sha256, + layout_order_from_bbox as _order_from_bbox, parse_json_object, prepare_image_for_api, preview, @@ -1005,21 +1007,6 @@ def _page_size_for_bbox(deck: Deck, page: SlidePage | None) -> tuple[float | Non return None -def _clamp_bbox(bbox: list[float]) -> list[float]: - x1, y1, x2, y2 = [max(0.0, min(1.0, float(value))) for value in bbox] - if x2 < x1: - x1, x2 = x2, x1 - if y2 < y1: - y1, y2 = y2, y1 - return [round(x1, 4), round(y1, 4), round(x2, 4), round(y2, 4)] - - -def _order_from_bbox(bbox: list[float] | None) -> float: - if not bbox: - return 9999.0 - return round(float(bbox[1]) * 1000.0 + float(bbox[0]), 4) - - def _layout_order(block: dict[str, Any]) -> float: try: return float(block.get("layout_order")) diff --git a/slidenote/study_pack/__init__.py b/slidenote/study_pack/__init__.py index 25c5dbb..924bfe9 100644 --- a/slidenote/study_pack/__init__.py +++ b/slidenote/study_pack/__init__.py @@ -1,6 +1,5 @@ from __future__ import annotations -import html import json import re from pathlib import Path @@ -11,7 +10,7 @@ from slidenote.llm import LLMClient, resolve_provider_runtime from slidenote.llm_cache import LLM_CACHE_SCHEMA_VERSION, LLMCache, make_cache_key, sha256_text, stable_json, utc_now_iso from slidenote.models import Deck, TableBlock, TextBlock -from slidenote.utils import as_float, display_path, round_score +from slidenote.utils import as_float, display_path from .common import ( IMPORTANCE_LABELS, diff --git a/slidenote/study_pack/exam.py b/slidenote/study_pack/exam.py index b7b9789..01bec52 100644 --- a/slidenote/study_pack/exam.py +++ b/slidenote/study_pack/exam.py @@ -1,7 +1,6 @@ from typing import Any import html import json -import re from .common import ( QUESTION_TYPE_LABELS, _as_int, diff --git a/slidenote/study_pack/questions.py b/slidenote/study_pack/questions.py index 6e1e981..e9636bb 100644 --- a/slidenote/study_pack/questions.py +++ b/slidenote/study_pack/questions.py @@ -1,5 +1,5 @@ -from slidenote.models import Deck, TableBlock, TextBlock -from slidenote.utils import as_float, display_path, round_score +from slidenote.models import Deck +from slidenote.utils import round_score from typing import Any import re from .common import ( diff --git a/slidenote/utils.py b/slidenote/utils.py index 17cfaf1..85136fe 100644 --- a/slidenote/utils.py +++ b/slidenote/utils.py @@ -188,6 +188,21 @@ def bbox_area(bbox: list[float]) -> float: return max(0.0, bbox[2] - bbox[0]) * max(0.0, bbox[3] - bbox[1]) +def clamp_normalized_bbox(bbox: list[float]) -> list[float]: + x1, y1, x2, y2 = [max(0.0, min(1.0, float(value))) for value in bbox] + if x2 < x1: + x1, x2 = x2, x1 + if y2 < y1: + y1, y2 = y2, y1 + return [round(x1, 4), round(y1, 4), round(x2, 4), round(y2, 4)] + + +def layout_order_from_bbox(bbox: list[float] | None) -> float: + if not bbox: + return 9999.0 + return round(float(bbox[1]) * 1000.0 + float(bbox[0]), 4) + + def file_sha256(path: Path) -> str: return "sha256:" + hashlib.sha256(path.read_bytes().hex().encode("utf-8")).hexdigest() diff --git a/tests/test_audit_redundancy.py b/tests/test_audit_redundancy.py new file mode 100644 index 0000000..3c4c04a --- /dev/null +++ b/tests/test_audit_redundancy.py @@ -0,0 +1,21 @@ +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + + +def test_redundancy_audit_avoids_known_false_positives(): + root = Path(__file__).resolve().parents[1] + completed = subprocess.run( + [sys.executable, str(root / "scripts" / "audit_redundancy.py")], + cwd=root, + check=False, + capture_output=True, + text=True, + ) + + assert completed.returncode == 0, completed.stderr + assert "unused import 'annotations'" not in completed.stdout + assert "\n slidenote.cli\n" not in completed.stdout + assert "slidenote.parser_adapters.supports == slidenote.parser_adapters.supports" not in completed.stdout diff --git a/tests/test_notes.py b/tests/test_notes.py index 2469426..c4d964f 100644 --- a/tests/test_notes.py +++ b/tests/test_notes.py @@ -1205,6 +1205,72 @@ class Result: assert result.llm_usage["summary"]["repair_contexts"] == 1 +def test_note_options_use_one_resolved_runtime_for_generation_repair_and_usage(tmp_path, monkeypatch): + deck = Deck( + source_path="lecture.pdf", + source_type="pdf", + pages=[ + SlidePage( + slide_id=1, + title="Quorum", + text_blocks=[TextBlock(id="s1_t1", type="paragraph", content="Quorum requires intersecting sets.")], + ) + ], + ) + content_guard = { + "required_confidence_threshold": 0.7, + "summary": {"repair_attempts": 0, "required_missing": 0, "residual_risks": 0}, + "pages": [{"slide_id": 1, "page_role": "content", "items": []}], + "items": [ + { + "element_id": "s1_t1", + "slide_id": 1, + "learning_role": "definition", + "must_explain": True, + "confidence": 0.95, + "reason": "definition", + } + ], + "repairs": [], + } + client_options = [] + + class FakeClient: + def __init__(self, **kwargs): + client_options.append(kwargs) + + def generate_with_usage(self, prompt): + class Result: + usage = {} + + result = Result() + if '"task": "repair_required_learning_coverage"' in prompt: + result.text = "Quorum requires intersecting read and write sets. " + else: + result.text = "" + return result + + monkeypatch.setattr("slidenote.notes.llm_calls.LLMClient", FakeClient) + + result = generate_notes_result( + deck, + tmp_path, + use_llm=True, + provider="chatgpt", + api_key="test", + note_strategy="direct", + note_context="page", + content_guard=content_guard, + ) + + assert len(client_options) == 2 + assert {item["provider"] for item in client_options} == {"openai"} + assert {item["model"] for item in client_options} == {"gpt-4.1-mini"} + assert result.llm_usage["provider"] == "openai" + assert result.llm_usage["model"] == "gpt-4.1-mini" + assert result.llm_usage["cache"]["dir"] == ".cache/llm" + + def test_section_context_notes_use_numbered_outline_headings(tmp_path, monkeypatch): deck = Deck( source_path="ch06.pdf", diff --git a/tests/test_parser_adapters.py b/tests/test_parser_adapters.py index fadfb08..852f1f6 100644 --- a/tests/test_parser_adapters.py +++ b/tests/test_parser_adapters.py @@ -2,12 +2,16 @@ import json import random import subprocess +import sys +import threading +import time +from concurrent.futures import ThreadPoolExecutor import fitz from PIL import Image from slidenote.extractors import extract_deck -from slidenote.extractors.pdf import extract_pdf +from slidenote.extractors.pdf import _extract_tables, extract_pdf from slidenote.models import Deck, SlidePage from slidenote.parser_adapters import available_parser_choices, parser_adapter_infos, parser_adapters @@ -74,6 +78,42 @@ def test_extract_pdf_parallel_keeps_page_order_and_content(tmp_path): assert any(block.content.startswith("Body line") for block in deck.pages[2].text_blocks) +def test_parallel_table_extraction_restores_process_streams(): + first_started = threading.Event() + + class Found: + tables = [] + + class FakePage: + def __init__(self, first: bool): + self.first = first + + def find_tables(self): + if self.first: + first_started.set() + time.sleep(0.05) + else: + time.sleep(0.1) + return Found() + + original_stdout = sys.stdout + original_stderr = sys.stderr + streams_restored = False + try: + with ThreadPoolExecutor(max_workers=2) as executor: + first = executor.submit(_extract_tables, FakePage(True), 1) + assert first_started.wait(timeout=1) + second = executor.submit(_extract_tables, FakePage(False), 2) + first.result() + second.result() + streams_restored = sys.stdout is original_stdout and sys.stderr is original_stderr + finally: + sys.stdout = original_stdout + sys.stderr = original_stderr + + assert streams_restored + + def test_builtin_parser_adapter_delegates_by_suffix(tmp_path, monkeypatch): source = tmp_path / "lecture.pdf" source.write_bytes(b"%PDF") diff --git a/tests/test_studio_core.py b/tests/test_studio_core.py index fb02df7..e43f690 100644 --- a/tests/test_studio_core.py +++ b/tests/test_studio_core.py @@ -1,5 +1,6 @@ from __future__ import annotations +import ast import os import sys from pathlib import Path @@ -101,6 +102,14 @@ def test_textbook_command_uses_env_for_ocr_keys(tmp_path: Path): assert env["BAIDU_OCR_SECRET_KEY"] == "ocr-secret" +def test_command_display_redacts_secret_flags_defensively(): + display = command_for_display( + ["slidenote", "build", "lecture.pdf", "--api-key", "text-secret", "--vision-api-key", "vision-secret"] + ) + + assert display == "slidenote build lecture.pdf --api-key *** --vision-api-key ***" + + def test_discover_outputs_includes_markdown_zip_and_exports(tmp_path: Path): for filename in ("notes.zip", "notes.toc.md", "notes.docx", "notes.pdf", "notes.tex"): (tmp_path / filename).write_bytes(b"x") @@ -196,6 +205,20 @@ def test_gui_workbench_surface_replaces_hero_cards(): assert "_render_empty_upload_panel" in source +def test_gui_app_invokes_main_when_streamlit_executes_the_script(): + source = (Path(__file__).resolve().parents[1] / "gui" / "app.py").read_text(encoding="utf-8") + tree = ast.parse(source) + final_statement = tree.body[-1] + + assert isinstance(final_statement, ast.If) + assert isinstance(final_statement.test, ast.Compare) + assert ast.unparse(final_statement.test) == "__name__ == '__main__'" + assert any( + isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == "main" + for node in ast.walk(final_statement) + ) + + def test_gui_workbench_file_size_helper(): pytest.importorskip("streamlit") from gui.app import _format_file_size