Attachments: bound page renders, resolve once, no images for blind models

From review. A PDF page rendered at 150 dpi had no size bound: a 3000 pt
page came out 6250x6250 at 171 MB, past the 20 MB cap an image file has;
renders are now scaled to 2048 and refused over the cap. Images resolved
on every tool round re-read files and re-rendered pages between rounds,
and could flip to gone mid-answer; they resolve once per answer. A
conversation holding images, reopened with a text-only model loaded, sent
image parts the server rejected; the references become a note instead.
Scanned pages past the fourth were dropped silently; they are listed as
not shown and the chip is truncated. A pdftotext timeout or a missing
poppler is a refusal, newlines in a name no longer close the file block,
an image is hashed from the bytes it sends, and /models is read once per
answer.

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

1 files changed, 82 insertions, 37 deletions

diff --git a/assistant.py b/assistant.py
index 7b95255..6d054b8 100644
--- a/assistant.py
+++ b/assistant.py
@@ -261,8 +261,15 @@ def sha256(path):
 
 
 def pdf_pages(path):
-    out = subprocess.run(["pdftotext", "-layout", path, "-"], capture_output=True,
-                         text=True, timeout=60)
+    # ponytail: the whole text is read, then cut to the share; a page cap
+    # (-l) if huge PDFs ever matter.
+    try:
+        out = subprocess.run(["pdftotext", "-layout", path, "-"], capture_output=True,
+                             text=True, timeout=60)
+    except subprocess.TimeoutExpired:
+        raise Refused(f"cannot read {os.path.basename(path)}: timed out") from None
+    except OSError as e:                  # poppler missing
+        raise Refused(f"cannot read {os.path.basename(path)}: {e.strerror}") from None
     if out.returncode:
         last = out.stderr.strip().splitlines()[-1:] or ["pdftotext failed"]
         raise Refused(f"cannot read {os.path.basename(path)}: {last[0]}")
@@ -278,7 +285,8 @@ def attach(paths, budget):
         raise Refused(f"at most {MAX_FILES} files")
     loaded = []
     for p in paths:
-        name = os.path.basename(p) or p
+        # A newline in a name would close the file block early.
+        name = " ".join((os.path.basename(p) or p).splitlines())
         if not (os.path.isabs(p) and os.path.isfile(p) and os.access(p, os.R_OK)):
             raise Refused(f"cannot read {name}")
         kind = kind_of(p)
@@ -300,7 +308,8 @@ def attach(paths, budget):
             chip["pages"] = len(pages)
             body = "\n\n".join(f"[page {i}]\n{t.rstrip()}" for i, t in enumerate(pages, 1))
             # A page with no text is a scan: the model reads it as an image.
-            blank = [i for i, t in enumerate(pages, 1) if not t.strip()][:MAX_PAGE_IMAGES]
+            scans = [i for i, t in enumerate(pages, 1) if not t.strip()]
+            blank, unseen = scans[:MAX_PAGE_IMAGES], scans[MAX_PAGE_IMAGES:]
             digest = sha256(p) if blank else ""
             refs += [{"type": "image_ref", "name": f"{name} page {n}", "path": p,
                       "sha256": digest, "page": n} for n in blank]
@@ -309,30 +318,40 @@ def attach(paths, budget):
                 body = f.read(share * 4 + 4).decode("utf-8", "replace")
         if len(body) > share:
             body, chip["truncated"] = body[:share], True
+        if kind == "pdf" and unseen:          # the model must not think it saw them all
+            body += f"\n[scanned pages not shown: {', '.join(map(str, unseen))}]"
+            chip["truncated"] = True
         note = " (truncated)" if chip["truncated"] else ""
         blocks.append(f"--- file: {name}{note} ---\n{body}\n--- end file ---")
     return blocks, refs, chips
 
 
-def resolve(part):
+def resolve(part, images=True):
     # An image reference back into an image the model can see, or a note
-    # that it is gone: moved, deleted, or changed since it was sent.
+    # that it is gone (moved, deleted, changed since it was sent) or that the
+    # model now loaded cannot see it, which would otherwise fail the request.
     if part.get("type") != "image_ref":
         return part
+    if not images:
+        return {"type": "text", "text": f"[image {part['name']}: this model cannot see images]"}
     gone = {"type": "text", "text": f"[image {part['name']} is no longer available]"}
     try:
-        if sha256(part["path"]) != part["sha256"]:
-            return gone
         if "page" in part:
+            if sha256(part["path"]) != part["sha256"]:
+                return gone
+            # Bounded: at -r 150 a large page renders huge (a 3000 pt page
+            # was 6250x6250 and 171 MB), past the cap an image file has.
             n = str(part["page"])
-            out = subprocess.run(["pdftoppm", "-f", n, "-l", n, "-r", "150", "-png", part["path"]],
-                                 capture_output=True, timeout=60)
-            if out.returncode or not out.stdout:
+            out = subprocess.run(["pdftoppm", "-f", n, "-l", n, "-r", "150", "-scale-to", "2048",
+                                  "-png", part["path"]], capture_output=True, timeout=60)
+            if out.returncode or not out.stdout or len(out.stdout) > MAX_IMAGE:
                 return gone
             data, mime = out.stdout, "image/png"
         else:
-            with open(part["path"], "rb") as f:
+            with open(part["path"], "rb") as f:   # once: the bytes hashed are the bytes sent
                 data = f.read()
+            if hashlib.sha256(data).hexdigest() != part["sha256"]:
+                return gone
             mime = mimetypes.guess_type(part["path"])[0]
     except (OSError, subprocess.TimeoutExpired):
         return gone
@@ -340,9 +359,9 @@ def resolve(part):
             "image_url": {"url": f"data:{mime};base64,{base64.b64encode(data).decode()}"}}
 
 
-def wire(messages):
+def wire(messages, images=True):
     # What is sent: the stored messages with their image references resolved.
-    return [{**m, "content": [resolve(p) for p in m["content"]]}
+    return [{**m, "content": [resolve(p, images) for p in m["content"]]}
             if isinstance(m.get("content"), list) else m for m in messages]
 
 
@@ -605,23 +624,19 @@ def collect_calls(calls, fragments):
         c["arguments"] += fn.get("arguments") or ""
 
 
-def llm_model():
-    # The router keeps one model resident; asking for another would swap it
-    # out under other clients, so use whatever is not unloaded.
+def model_inputs():
+    # (id, inputs) of the model in use. The router keeps one model resident;
+    # asking for another would swap it out under other clients, so use
+    # whatever is not unloaded. inputs is None when the server does not
+    # report them.
     with urllib.request.urlopen(CFG["llm"] + "/models", timeout=10) as r:
         models = json.load(r)["data"]
-    return next((m["id"] for m in models if m["status"]["value"] != "unloaded"),
-                models[0]["id"])
+    m = next((m for m in models if m["status"]["value"] != "unloaded"), models[0])
+    return m["id"], (m.get("architecture") or {}).get("input_modalities")
 
 
-def sees_images(model):
-    # The router reports each model's inputs; a server that does not is
-    # left to decide for itself.
-    with urllib.request.urlopen(CFG["llm"] + "/models", timeout=10) as r:
-        models = json.load(r)["data"]
-    m = next((m for m in models if m["id"] == model), {})
-    inputs = (m.get("architecture") or {}).get("input_modalities")
-    return inputs is None or "image" in inputs
+def llm_model():
+    return model_inputs()[0]
 
 
 def ask(messages, calls, tools=None):
@@ -712,9 +727,11 @@ def answer(mine, text, files=()):
         # Half of what the context has free, at about 3 characters a token.
         free = (ctx["max"] - ctx["used"]) // 2 * 3 if ctx["max"] else 24000
         blocks, refs, chips = attach(files, max(free, 2000))
-        model = llm_model()
-        if refs and not sees_images(model):
-            raise Refused(f"{model} cannot see images")
+    # A server that does not report inputs is left to decide for itself.
+    model, inputs = model_inputs()
+    images = inputs is None or "image" in inputs
+    if refs and not images:
+        raise Refused(f"{model} cannot see images")
     with lock:
         if turn != mine:
             return
@@ -737,13 +754,17 @@ def answer(mine, text, files=()):
             + [{"role": "user", "content": [{"type": "text", "text": f"[{now}] {body}"}] + refs
                 if refs else f"[{now}] {body}"}])
     first = len(msgs)                            # this turn's tool messages start here
+    # Resolved once per answer: images re-read and pages re-rendered on every
+    # tool round would cost seconds between rounds, and a file changed
+    # mid-answer would flip from image to gone.
+    sent = wire(msgs, images)
     said, used = "", []
     # ponytail: a stopped reply notices at its next chunk, so llama-server's
     # slot stays busy until then and the next question queues behind it.
     for rnd in range(CFG["tool_rounds"] + 1):
         calls, content = {}, ""
         # The last round offers no tools, which forces an answer.
-        for kind, piece in ask(wire(msgs), calls,
+        for kind, piece in ask(sent, calls,
                                TOOL_SCHEMA if rnd < CFG["tool_rounds"] else None):
             if not say(mine, kind, piece):
                 log("interrupted")
@@ -753,10 +774,12 @@ def answer(mine, text, files=()):
         said += content
         if not calls:
             break
-        msgs.append({"role": "assistant", "content": content, "tool_calls": [
+        call = {"role": "assistant", "content": content, "tool_calls": [
             {"id": c["id"], "type": "function",
              "function": {"name": c["name"], "arguments": c["arguments"]}}
-            for c in calls.values()]})
+            for c in calls.values()]}
+        msgs.append(call)
+        sent.append(call)
         # ponytail: a stop does not kill a running tool, its result is dropped
         for c in calls.values():
             if not say(mine, "tool", c["name"]):
@@ -766,6 +789,7 @@ def answer(mine, text, files=()):
             result = run_tool(TOOLS, c["name"], c["arguments"])
             log(f"tool {c['name']}({c['arguments']}): {result[:120]!r}")
             msgs.append({"role": "tool", "tool_call_id": c["id"], "content": result})
+            sent.append(msgs[-1])
             if not say(mine, "tool", c["name"], done=True):
                 log("interrupted")
                 return
@@ -908,14 +932,16 @@ def test():
     import zlib
 
     def tiny_pdf(path, pages):
-        # One Helvetica line per page; "" is a page with no text, a scan.
+        # One Helvetica line per page; "" is a page with no text, a scan. A
+        # page may be (text, width, height) in points.
         objs = ["<< /Type /Catalog /Pages 2 0 R >>",
                 "<< /Type /Pages /Kids [%s] /Count %d >>"
                 % (" ".join(f"{4 + 2 * i} 0 R" for i in range(len(pages))), len(pages)),
                 "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>"]
         for i, t in enumerate(pages):
+            t, w, h = t if isinstance(t, tuple) else (t, 612, 792)
             stream = f"BT /F1 24 Tf 72 700 Td ({t}) Tj ET" if t else ""
-            objs += [f"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources "
+            objs += [f"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 {w} {h}] /Resources "
                      f"<< /Font << /F1 3 0 R >> >> /Contents {5 + 2 * i} 0 R >>",
                      f"<< /Length {len(stream)} >>\nstream\n{stream}\nendstream"]
         out, offsets = b"%PDF-1.4\n", []
@@ -943,7 +969,7 @@ def test():
     with open(code, "w") as f:
         f.write("print('hi')\n" * 50)
     tiny_png(png)
-    tiny_pdf(pdf, ["hello pdf", ""])
+    tiny_pdf(pdf, ["hello pdf", ("", 3000, 3000)])
     with open(other, "wb") as f:
         f.write(b"\x00\x01")
     assert [kind_of(p) for p in (code, png, pdf, other, "/x/README.md", "/x/b.SlackBuild")] == \
@@ -959,7 +985,24 @@ def test():
     sent = [resolve(r) for r in refs]
     assert all(x["type"] == "image_url" for x in sent)
     assert sent[1]["image_url"]["url"].startswith("data:image/png;base64,")
-    assert base64.b64decode(sent[0]["image_url"]["url"].split(",", 1)[1])[:4] == b"\x89PNG"
+    page = base64.b64decode(sent[0]["image_url"]["url"].split(",", 1)[1])
+    # Page 2, the square one, bounded to 2048: page 1 would not be square.
+    assert page[:4] == b"\x89PNG" and struct.unpack(">II", page[16:24]) == (2048, 2048)
+    scans = os.path.join(fd, "scans.pdf")
+    tiny_pdf(scans, ["cover"] + [""] * 6)
+    b, r, c = attach([scans], 100000)
+    assert len(r) == 4 and c[0]["truncated"] and "[scanned pages not shown: 6, 7]" in b[0]
+    with open(os.path.join(fd, "exact.txt"), "w") as f:
+        f.write("x" * 24)
+    with open(os.path.join(fd, "over.txt"), "w") as f:
+        f.write("x" * 25)
+    for n, cut in (("exact.txt", False), ("over.txt", True)):
+        b, _, c = attach([os.path.join(fd, n)], 24)
+        assert c[0]["truncated"] is cut and b[0].split("\n")[1] == "x" * 24, n
+    with open(os.path.join(fd, "a\nb.txt"), "w") as f:
+        f.write("x")
+    b, _, c = attach([os.path.join(fd, "a\nb.txt")], 100)
+    assert c[0]["name"] == "a b.txt" and b[0].startswith("--- file: a b.txt ---\n")
     small = attach([code], 24)[0][0]                 # the share is the cut: two lines
     assert "(truncated)" in small and small.count("print") == 2
     for bad, why in ((["a.py"], "cannot read a.py"), ([fd + "/nope.txt"], "cannot read nope.txt"),
@@ -976,6 +1019,8 @@ def test():
     hist, shown = load_conv(db, a)
     assert shown[0]["files"] == chips[2:] and list_convs(db)[0]["title"] == "red.png"
     assert wire(hist[0])[0]["content"][0]["type"] == "image_url"
+    assert wire(hist[0], images=False)[0]["content"][0] == {   # a text-only model now
+        "type": "text", "text": "[image red.png: this model cannot see images]"}
     tiny_png(png, b"\x00\xff\x00")
     assert wire(hist[0])[0]["content"][0] == {
         "type": "text", "text": "[image red.png is no longer available]"}