Refactor remaining flagged items for clean self-analysis

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: clanker-lover

codedocent/editor.py

@@ -6,6 +6,41 @@
 import shutil
 
 
+def _read_and_validate(
+    filepath: str, start_line: int, end_line: int,
+) -> tuple[list[str] | None, str | None]:
+    """Read *filepath* and validate the line range.
+
+    Returns ``(lines, None)`` on success, or ``(None, error_message)``
+    on failure.
+    """
+    if not os.path.isfile(filepath):
+        return (None, f"File not found: {filepath}")
+    if (
+        not isinstance(start_line, int)
+        or not isinstance(end_line, int)
+        or start_line < 1
+        or end_line < 1
+        or start_line > end_line
+    ):
+        return (None, f"Invalid line range: {start_line}-{end_line}")
+    with open(filepath, encoding="utf-8") as f:
+        lines = f.readlines()
+    if end_line > len(lines):
+        return (
+            None,
+            f"end_line {end_line} exceeds file length ({len(lines)} lines)",
+        )
+    return (lines, None)
+
+
+def _write_with_backup(filepath: str, lines: list[str]) -> None:
+    """Create a ``.bak`` backup and write *lines* back to *filepath*."""
+    shutil.copy2(filepath, filepath + ".bak")
+    with open(filepath, "w", encoding="utf-8") as f:
+        f.writelines(lines)
+
+
 def replace_block_source(
     filepath: str,
     start_line: int,
@@ -18,45 +53,16 @@ def replace_block_source(
     ``success``, ``lines_before``, ``lines_after`` on success, or
     ``success=False`` and ``error`` on failure.
     """
-    # --- input validation ---
-    if not os.path.isfile(filepath):
-        return {"success": False, "error": f"File not found: {filepath}"}
-
-    if (
-        not isinstance(start_line, int)
-        or not isinstance(end_line, int)
-        or start_line < 1
-        or end_line < 1
-        or start_line > end_line
-    ):
-        return {
-            "success": False,
-            "error": (
-                f"Invalid line range: {start_line}-{end_line}"
-            ),
-        }
-
     if not isinstance(new_source, str):
         return {"success": False, "error": "new_source must be a string"}
 
-    try:
-        with open(filepath, encoding="utf-8") as f:
-            lines = f.readlines()
-
-        if end_line > len(lines):
-            return {
-                "success": False,
-                "error": (
-                    f"end_line {end_line} exceeds file length"
-                    f" ({len(lines)} lines)"
-                ),
-            }
+    lines, error = _read_and_validate(filepath, start_line, end_line)
+    if lines is None:
+        return {"success": False, "error": error}
 
-        old_count = end_line - start_line + 1
-
-        # Backup
-        shutil.copy2(filepath, filepath + ".bak")
+    old_count = end_line - start_line + 1
 
+    try:
         # Build replacement lines
         if new_source == "":
             new_lines: list[str] = []
@@ -69,11 +75,9 @@ def replace_block_source(
             new_lines = [ln + "\n" for ln in new_lines]
 
         new_count = len(new_lines)
-
         lines[start_line - 1:end_line] = new_lines
 
-        with open(filepath, "w", encoding="utf-8") as f:
-            f.writelines(lines)
+        _write_with_backup(filepath, lines)
 
         return {
             "success": True,

codedocent/gui.py

@@ -20,23 +20,15 @@
 _fetch_ollama_models = fetch_ollama_models
 
 
-def _build_gui() -> None:
-    """Build and run the tkinter GUI window."""
-    root = tk.Tk()
-    root.title("codedocent")
-    root.resizable(False, False)
-
-    frame = ttk.Frame(root, padding=16)
-    frame.grid(row=0, column=0, sticky="nsew")
-
-    # --- Folder picker ---
+def _create_folder_row(frame: ttk.Frame) -> tk.StringVar:
+    """Create the folder-picker row and return the StringVar."""
     ttk.Label(frame, text="Folder to analyze:").grid(
         row=0, column=0, sticky="w", pady=(0, 4),
     )
-
     folder_var = tk.StringVar()
-    folder_entry = ttk.Entry(frame, textvariable=folder_var, width=40)
-    folder_entry.grid(row=1, column=0, sticky="ew", padx=(0, 6))
+    ttk.Entry(frame, textvariable=folder_var, width=40).grid(
+        row=1, column=0, sticky="ew", padx=(0, 6),
+    )
 
     def _browse() -> None:
         path = filedialog.askdirectory()
@@ -46,46 +38,50 @@ def _browse() -> None:
     ttk.Button(frame, text="Browse...", command=_browse).grid(
         row=1, column=1, sticky="w",
     )
+    return folder_var
+
 
-    # --- Model dropdown ---
+def _create_model_row(frame: ttk.Frame) -> tk.StringVar:
+    """Create the model-dropdown row and return the StringVar."""
     ttk.Label(frame, text="Model:").grid(
         row=2, column=0, sticky="w", pady=(12, 4),
     )
-
     ollama_ok = _check_ollama()
     models = _fetch_ollama_models() if ollama_ok else []
-
     model_values = models if models else ["No AI"]
     model_var = tk.StringVar(value=model_values[0])
-    model_combo = ttk.Combobox(
+    ttk.Combobox(
         frame, textvariable=model_var, values=model_values,
         state="readonly", width=37,
-    )
-    model_combo.grid(row=3, column=0, columnspan=2, sticky="ew")
+    ).grid(row=3, column=0, columnspan=2, sticky="ew")
+    return model_var
 
-    # --- Mode selector ---
+
+def _create_mode_row(frame: ttk.Frame) -> tk.StringVar:
+    """Create the mode-selector row and return the StringVar."""
     ttk.Label(frame, text="Mode:").grid(
         row=4, column=0, sticky="w", pady=(12, 4),
     )
-
     mode_var = tk.StringVar(value="interactive")
     modes_frame = ttk.Frame(frame)
     modes_frame.grid(row=5, column=0, columnspan=2, sticky="w")
-
-    ttk.Radiobutton(
-        modes_frame, text="Interactive", variable=mode_var,
-        value="interactive",
-    ).pack(anchor="w")
-    ttk.Radiobutton(
-        modes_frame, text="Full export", variable=mode_var,
-        value="full",
-    ).pack(anchor="w")
-    ttk.Radiobutton(
-        modes_frame, text="Text tree", variable=mode_var,
-        value="text",
-    ).pack(anchor="w")
-
-    # --- Go button ---
+    for text, value in [("Interactive", "interactive"),
+                        ("Full export", "full"),
+                        ("Text tree", "text")]:
+        ttk.Radiobutton(
+            modes_frame, text=text, variable=mode_var, value=value,
+        ).pack(anchor="w")
+    return mode_var
+
+
+def _create_go_button(
+    frame: ttk.Frame,
+    root: tk.Tk,
+    folder_var: tk.StringVar,
+    model_var: tk.StringVar,
+    mode_var: tk.StringVar,
+) -> None:
+    """Create the Go button with its launch logic."""
     def _go() -> None:
         folder = folder_var.get().strip()
         if not folder:
@@ -112,6 +108,21 @@ def _go() -> None:
         row=6, column=0, columnspan=2, pady=(16, 0),
     )
 
+
+def _build_gui() -> None:
+    """Build and run the tkinter GUI window."""
+    root = tk.Tk()
+    root.title("codedocent")
+    root.resizable(False, False)
+
+    frame = ttk.Frame(root, padding=16)
+    frame.grid(row=0, column=0, sticky="nsew")
+
+    folder_var = _create_folder_row(frame)
+    model_var = _create_model_row(frame)
+    mode_var = _create_mode_row(frame)
+    _create_go_button(frame, root, folder_var, model_var, mode_var)
+
     root.mainloop()
 
 

codedocent/server.py

@@ -120,6 +120,52 @@ def _idle_watcher():
     watcher.start()
 
 
+def _analyze_node(node_id: str) -> dict | None:
+    """Look up *node_id* and return an analyzed dict, or None if not found."""
+    if node_id not in _Handler.node_lookup:
+        return None
+    node = _Handler.node_lookup[node_id]
+    if node.summary is not None:
+        return _node_to_dict(node, include_source=True)
+    with _Handler.analyze_lock:
+        if node.summary is None:
+            from codedocent.analyzer import analyze_single_node  # pylint: disable=import-outside-toplevel  # noqa: E501
+
+            analyze_single_node(node, _Handler.model, _Handler.cache_dir)
+    return _node_to_dict(node, include_source=True)
+
+
+def _execute_replace(node_id: str, body: dict) -> tuple[int, dict]:
+    """Validate and execute a source-replacement request.
+
+    Returns ``(status_code, result_dict)``.
+    """
+    if node_id not in _Handler.node_lookup:
+        return (404, {"success": False, "error": "Unknown node ID"})
+    node = _Handler.node_lookup[node_id]
+    if node.node_type in ("directory", "file"):
+        return (
+            400,
+            {"success": False,
+             "error": "Cannot replace directory/file blocks"},
+        )
+    new_source = body.get("source", "")
+    if not isinstance(new_source, str):
+        return (400, {"success": False, "error": "source must be a string"})
+    abs_path = _resolve_filepath(node, _Handler.cache_dir)
+    from codedocent.editor import replace_block_source  # pylint: disable=import-outside-toplevel  # noqa: E501
+
+    with _Handler.analyze_lock:
+        result = replace_block_source(
+            abs_path, node.start_line, node.end_line, new_source,
+        )
+        if result["success"]:
+            _update_node_after_replace(
+                node, new_source, result, _Handler.cache_dir,
+            )
+    return (200, result)
+
+
 class _Handler(BaseHTTPRequestHandler):
     """HTTP request handler for codedocent server."""
 
@@ -175,101 +221,30 @@ def _serve_html(self):
         self.wfile.write(data)
 
     def _serve_tree(self):
-        tree_dict = _node_to_dict(_Handler.root)
-        data = json.dumps(tree_dict).encode("utf-8")
-        self.send_response(200)
-        self.send_header("Content-Type", "application/json")
-        self.send_header("Content-Length", str(len(data)))
-        self.end_headers()
-        self.wfile.write(data)
+        self._send_json(200, _node_to_dict(_Handler.root))
 
     def _handle_source(self, node_id: str):
         if node_id not in _Handler.node_lookup:
             self.send_error(404, "Unknown node ID")
             return
         node = _Handler.node_lookup[node_id]
-        result = {"source": node.source or ""}
-        data = json.dumps(result).encode("utf-8")
-        self.send_response(200)
-        self.send_header("Content-Type", "application/json")
-        self.send_header("Content-Length", str(len(data)))
-        self.end_headers()
-        self.wfile.write(data)
+        self._send_json(200, {"source": node.source or ""})
 
     def _handle_analyze(self, node_id: str):
-        if node_id not in _Handler.node_lookup:
+        result = _analyze_node(node_id)
+        if result is None:
             self.send_error(404, "Unknown node ID")
             return
-
-        node = _Handler.node_lookup[node_id]
-
-        # Return cached result if already analyzed
-        if node.summary is not None:
-            result = _node_to_dict(node, include_source=True)
-            data = json.dumps(result).encode("utf-8")
-            self.send_response(200)
-            self.send_header("Content-Type", "application/json")
-            self.send_header("Content-Length", str(len(data)))
-            self.end_headers()
-            self.wfile.write(data)
-            return
-
-        # Run analysis (thread-safe)
-        with _Handler.analyze_lock:
-            # Double-check after acquiring lock
-            if node.summary is None:
-                from codedocent.analyzer import analyze_single_node  # pylint: disable=import-outside-toplevel  # noqa: E501
-
-                analyze_single_node(node, _Handler.model, _Handler.cache_dir)
-
-        result = _node_to_dict(node, include_source=True)
-        data = json.dumps(result).encode("utf-8")
-        self.send_response(200)
-        self.send_header("Content-Type", "application/json")
-        self.send_header("Content-Length", str(len(data)))
-        self.end_headers()
-        self.wfile.write(data)
+        self._send_json(200, result)
 
     def _handle_replace(self, node_id: str):
-        if node_id not in _Handler.node_lookup:
-            self.send_error(404, "Unknown node ID")
-            return
-
-        node = _Handler.node_lookup[node_id]
-
-        if node.node_type in ("directory", "file"):
-            self._send_json(
-                400,
-                {"success": False,
-                 "error": "Cannot replace directory/file blocks"},
-            )
-            return
-
         content_length = int(self.headers["Content-Length"])
         body = json.loads(self.rfile.read(content_length))
-        new_source = body.get("source", "")
-
-        if not isinstance(new_source, str):
-            self._send_json(
-                400,
-                {"success": False, "error": "source must be a string"},
-            )
+        status, result = _execute_replace(node_id, body)
+        if status == 404:
+            self.send_error(404, result["error"])
             return
-
-        abs_path = _resolve_filepath(node, _Handler.cache_dir)
-
-        from codedocent.editor import replace_block_source  # pylint: disable=import-outside-toplevel  # noqa: E501
-
-        with _Handler.analyze_lock:
-            result = replace_block_source(
-                abs_path, node.start_line, node.end_line, new_source,
-            )
-            if result["success"]:
-                _update_node_after_replace(
-                    node, new_source, result, _Handler.cache_dir,
-                )
-
-        self._send_json(200, result)
+        self._send_json(status, result)
 
     def _send_json(self, status_code: int, obj: dict):
         data = json.dumps(obj).encode("utf-8")