mirror of
https://github.com/lllin000/PaperForge.git
synced 2026-07-22 06:50:53 +00:00
fix: close 4 review gaps — registry, body/object-unit schemas, compat vector arm
Changes: - Add paper-lookup, content-discovery, scoped-fetch to command registry - Add unit_kind column to body_units schema (DEFAULT 'body') - Replace object_units schema: object_role -> object_kind, object_label, caption_text, nearby_body_text; drop unit_text, drop object_role - Bump schema version to 3 with deterministic drop/recreate migration for derived tables (body_units, body_units_fts, object_units) - Update _upsert_body_units / _upsert_object_units for new columns - Update build_object_units to emit object_kind, object_label, caption_text, nearby_body_text fields - Add vector secondary arm to _run_compat_content_discovery: runs paper_fts primary, conditionally adds vector_retrieve secondary when vector is healthy - Add/fix tests for schema columns, builder output, registry, and compat content-discovery vector-secondary path
This commit is contained in:
parent
88838c1842
commit
c534e481c7
7 changed files with 200 additions and 46 deletions
|
|
@ -19,6 +19,9 @@ _COMMAND_REGISTRY: dict[str, str] = {
|
|||
"prune": "paperforge.commands.prune",
|
||||
"paper-navigation": "paperforge.commands.paper_navigation",
|
||||
"reading-log": "paperforge.commands.reading_log",
|
||||
"paper-lookup": "paperforge.commands.paper_lookup",
|
||||
"content-discovery": "paperforge.commands.content_discovery",
|
||||
"scoped-fetch": "paperforge.commands.scoped_fetch",
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -370,14 +370,15 @@ def _upsert_body_units(conn: sqlite3.Connection, body_units: list[dict]) -> None
|
|||
conn.execute(
|
||||
"""INSERT OR REPLACE INTO body_units
|
||||
(unit_id, paper_id, section_path, unit_text,
|
||||
page_span_json, block_span_json, token_estimate,
|
||||
indexable, veto_reason, quality_hints_json)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
unit_kind, page_span_json, block_span_json,
|
||||
token_estimate, indexable, veto_reason, quality_hints_json)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
(
|
||||
unit["unit_id"],
|
||||
unit["paper_id"],
|
||||
unit["section_path"],
|
||||
unit["unit_text"],
|
||||
unit.get("unit_kind", "body"),
|
||||
json.dumps(unit.get("page_span", [])),
|
||||
json.dumps(unit.get("block_span", [])),
|
||||
unit.get("token_estimate", 0),
|
||||
|
|
@ -401,16 +402,19 @@ def _upsert_object_units(conn: sqlite3.Connection, object_units: list[dict]) ->
|
|||
for unit in object_units:
|
||||
conn.execute(
|
||||
"""INSERT OR REPLACE INTO object_units
|
||||
(unit_id, paper_id, section_path, unit_text,
|
||||
object_role, page_span_json, block_span_json,
|
||||
(unit_id, paper_id, section_path,
|
||||
object_kind, object_label, caption_text, nearby_body_text,
|
||||
page_span_json, block_span_json,
|
||||
token_estimate, indexable, veto_reason, quality_hints_json)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
(
|
||||
unit["unit_id"],
|
||||
unit["paper_id"],
|
||||
unit["section_path"],
|
||||
unit["unit_text"],
|
||||
unit.get("object_role", ""),
|
||||
unit.get("object_kind", ""),
|
||||
unit.get("object_label", ""),
|
||||
unit.get("caption_text", ""),
|
||||
unit.get("nearby_body_text", ""),
|
||||
json.dumps(unit.get("page_span", [])),
|
||||
json.dumps(unit.get("block_span", [])),
|
||||
unit.get("token_estimate", 0),
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import sqlite3
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
CURRENT_SCHEMA_VERSION = 2 # Bump from 1 for reading_log + project_log tables
|
||||
CURRENT_SCHEMA_VERSION = 3 # Bump from 2 for unit_kind + object_units schema changes
|
||||
|
||||
CREATE_META = """
|
||||
CREATE TABLE IF NOT EXISTS meta (
|
||||
|
|
@ -181,6 +181,7 @@ CREATE TABLE IF NOT EXISTS body_units (
|
|||
paper_id TEXT NOT NULL,
|
||||
section_path TEXT NOT NULL,
|
||||
unit_text TEXT NOT NULL,
|
||||
unit_kind TEXT NOT NULL DEFAULT 'body',
|
||||
page_span_json TEXT NOT NULL,
|
||||
block_span_json TEXT NOT NULL,
|
||||
token_estimate INTEGER NOT NULL,
|
||||
|
|
@ -206,8 +207,10 @@ CREATE TABLE IF NOT EXISTS object_units (
|
|||
unit_id TEXT PRIMARY KEY,
|
||||
paper_id TEXT NOT NULL,
|
||||
section_path TEXT NOT NULL,
|
||||
unit_text TEXT NOT NULL,
|
||||
object_role TEXT NOT NULL,
|
||||
object_kind TEXT NOT NULL,
|
||||
object_label TEXT NOT NULL,
|
||||
caption_text TEXT NOT NULL,
|
||||
nearby_body_text TEXT NOT NULL DEFAULT '',
|
||||
page_span_json TEXT NOT NULL,
|
||||
block_span_json TEXT NOT NULL,
|
||||
token_estimate INTEGER NOT NULL DEFAULT 0,
|
||||
|
|
@ -217,6 +220,7 @@ CREATE TABLE IF NOT EXISTS object_units (
|
|||
);
|
||||
"""
|
||||
|
||||
|
||||
ALL_TABLES = ["body_units", "body_units_fts", "object_units", "paper_fts", "reading_log", "project_log", "paper_events", "paper_assets", "paper_aliases", "papers", "meta"]
|
||||
|
||||
|
||||
|
|
@ -230,6 +234,14 @@ def ensure_schema(conn: sqlite3.Connection) -> None:
|
|||
conn.execute(CREATE_EVENTS)
|
||||
conn.execute(CREATE_READING_LOG)
|
||||
conn.execute(CREATE_PROJECT_LOG)
|
||||
|
||||
# Migration: derived tables are rebuildable, drop and recreate when shape changes
|
||||
current_version = get_schema_version(conn)
|
||||
if current_version < 3:
|
||||
logger.info("Migrating schema v%s -> v3: rebuilding body_units, body_units_fts, object_units", current_version)
|
||||
for table in ("body_units", "body_units_fts", "object_units"):
|
||||
conn.execute(f"DROP TABLE IF EXISTS {table};")
|
||||
|
||||
conn.execute(CREATE_BODY_UNITS)
|
||||
conn.execute(CREATE_BODY_UNITS_FTS)
|
||||
conn.execute(CREATE_OBJECT_UNITS)
|
||||
|
|
@ -239,6 +251,8 @@ def ensure_schema(conn: sqlite3.Connection) -> None:
|
|||
conn.execute(idx_sql)
|
||||
for trigger_sql in FTS_TRIGGERS:
|
||||
conn.execute(trigger_sql)
|
||||
|
||||
_set_schema_version(conn, CURRENT_SCHEMA_VERSION)
|
||||
conn.commit()
|
||||
|
||||
|
||||
|
|
@ -266,3 +280,11 @@ def get_schema_version(conn: sqlite3.Connection) -> int:
|
|||
return int(row["value"]) if row else 0
|
||||
except sqlite3.OperationalError:
|
||||
return 0
|
||||
|
||||
|
||||
def _set_schema_version(conn: sqlite3.Connection, version: int) -> None:
|
||||
"""Write the schema version into the meta table."""
|
||||
conn.execute(
|
||||
"INSERT OR REPLACE INTO meta (key, value) VALUES ('schema_version', ?)",
|
||||
(str(version),),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -5,11 +5,12 @@ route through. Each intent is routed to the correct real data source.
|
|||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
|
||||
from paperforge import __version__ as PF_VERSION
|
||||
from paperforge.core.io import read_json
|
||||
|
|
@ -248,7 +249,11 @@ def _run_compat_content_discovery(
|
|||
limit: int = 5,
|
||||
explanation_note: str = "",
|
||||
) -> PFResult:
|
||||
"""Fallback content discovery using ``paper_fts`` (metadata-only)."""
|
||||
"""Fallback content discovery using ``paper_fts`` (metadata-only).
|
||||
|
||||
When the vector arm is available, a secondary vector chunk result is
|
||||
included in the response alongside ``paper_fts`` as the primary arm.
|
||||
"""
|
||||
from paperforge.memory.fts import search_papers
|
||||
|
||||
db_path = get_memory_db_path(vault)
|
||||
|
|
@ -270,25 +275,10 @@ def _run_compat_content_discovery(
|
|||
conn = get_connection(db_path, read_only=True)
|
||||
try:
|
||||
results = search_papers(conn, query, limit=limit)
|
||||
return PFResult(
|
||||
ok=True,
|
||||
command="content-discovery",
|
||||
version=PF_VERSION,
|
||||
data={
|
||||
"intent": "content-discovery",
|
||||
"query": query,
|
||||
"results": results,
|
||||
"count": len(results),
|
||||
"route_explanation": {
|
||||
"primary_arm": "paper_fts",
|
||||
"compatibility_mode": True,
|
||||
"note": (
|
||||
explanation_note
|
||||
or "body_units_fts unavailable, using metadata FTS"
|
||||
),
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
# Secondary arm: vector retrieve if available
|
||||
secondary_arm: str | None = None
|
||||
vector_results = None
|
||||
except Exception as exc:
|
||||
return PFResult(
|
||||
ok=False,
|
||||
|
|
@ -304,6 +294,43 @@ def _run_compat_content_discovery(
|
|||
},
|
||||
},
|
||||
)
|
||||
else:
|
||||
try:
|
||||
from paperforge.embedding import get_embed_status, retrieve_chunks
|
||||
|
||||
status = get_embed_status(vault)
|
||||
if status.get("healthy") and status.get("chunk_count", 0) > 0:
|
||||
chunks = retrieve_chunks(vault, query, limit=limit)
|
||||
if chunks:
|
||||
secondary_arm = "vector_retrieve"
|
||||
vector_results = chunks
|
||||
except Exception:
|
||||
logger.warning("vector secondary arm unavailable in compat mode", exc_info=True)
|
||||
|
||||
route_explanation: dict[str, Any] = {
|
||||
"primary_arm": "paper_fts",
|
||||
"compatibility_mode": True,
|
||||
"note": (
|
||||
explanation_note
|
||||
or "body_units_fts unavailable, using metadata FTS"
|
||||
),
|
||||
}
|
||||
if secondary_arm:
|
||||
route_explanation["secondary_arm"] = secondary_arm
|
||||
|
||||
return PFResult(
|
||||
ok=True,
|
||||
command="content-discovery",
|
||||
version=PF_VERSION,
|
||||
data={
|
||||
"intent": "content-discovery",
|
||||
"query": query,
|
||||
"results": results,
|
||||
"count": len(results),
|
||||
"vector_results": vector_results,
|
||||
"route_explanation": route_explanation,
|
||||
},
|
||||
)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
|
|
|||
|
|
@ -72,20 +72,33 @@ def build_body_units(
|
|||
def build_object_units(
|
||||
*,
|
||||
tree: dict[str, Any],
|
||||
structured_blocks: list[dict[str, Any]], # kept for interface consistency
|
||||
structured_blocks: list[dict[str, Any]],
|
||||
role_index: dict[str, Any],
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Build object retrieval units from the role index scoped by section.
|
||||
|
||||
For each section node, objects (figures, tables, equations, …) whose
|
||||
``block_id`` falls within the node's ``block_span`` are collected from
|
||||
the role index and emitted as individual object units.
|
||||
the role index and emitted as individual object units with structured
|
||||
metadata fields (``object_kind``, ``object_label``, ``caption_text``,
|
||||
``nearby_body_text``).
|
||||
"""
|
||||
import re
|
||||
|
||||
units: list[dict[str, Any]] = []
|
||||
paper_id = tree.get("paper_id", "")
|
||||
for node in tree.get("nodes", []):
|
||||
block_ids = {block_id for _, block_id in node.get("block_span", [])}
|
||||
span = node.get("block_span", [])
|
||||
|
||||
# Pre-collect body text from this section for nearby_body_text
|
||||
body_texts = [
|
||||
b.get("text", "")
|
||||
for b in structured_blocks
|
||||
if b.get("block_id") in block_ids and b.get("role") == "body_paragraph"
|
||||
]
|
||||
nearby_body = "\n\n".join(t for t in body_texts if t)
|
||||
|
||||
for role, entries in role_index.items():
|
||||
for entry in entries:
|
||||
if not isinstance(entry, dict):
|
||||
|
|
@ -98,16 +111,32 @@ def build_object_units(
|
|||
node["page_span"][0], entry.get("block_id", ""),
|
||||
node["page_span"][1], entry.get("block_id", ""),
|
||||
)
|
||||
|
||||
# Map role to object_kind
|
||||
role_lower = role.lower()
|
||||
if role_lower.startswith("figure") or "figure" in role_lower:
|
||||
object_kind = "figure"
|
||||
elif role_lower.startswith("table") or "table" in role_lower:
|
||||
object_kind = "table"
|
||||
else:
|
||||
object_kind = role
|
||||
|
||||
# Extract object_label from caption prefix
|
||||
label_match = re.match(r'(Figure|Fig\.?\s*|Table|Tab\.?\s*)\s*\d+', text, re.IGNORECASE)
|
||||
object_label = label_match.group(0) if label_match else entry.get("block_id", "")
|
||||
|
||||
unit = {
|
||||
"unit_id": uid,
|
||||
"paper_id": paper_id,
|
||||
"section_path": "/".join(node.get("section_path", [])),
|
||||
"page_span": node.get("page_span", []),
|
||||
"block_span": span,
|
||||
"unit_text": text,
|
||||
"object_kind": object_kind,
|
||||
"object_label": object_label,
|
||||
"caption_text": text,
|
||||
"nearby_body_text": nearby_body,
|
||||
"token_estimate": len(text) // 4,
|
||||
"unit_kind": "object",
|
||||
"object_role": role,
|
||||
"indexable": bool(text.strip()),
|
||||
"veto_reason": "" if text.strip() else "empty",
|
||||
"quality_hints": [],
|
||||
|
|
|
|||
|
|
@ -444,3 +444,33 @@ def test_all_intents_surface_route_explanation(tmp_path):
|
|||
assert "primary_arm" in result.data["route_explanation"], (
|
||||
f"{intent} missing primary_arm"
|
||||
)
|
||||
|
||||
|
||||
def test_command_registry_contains_gateway_commands():
|
||||
from paperforge.commands import _COMMAND_REGISTRY
|
||||
|
||||
for name in ("paper-lookup", "content-discovery", "scoped-fetch"):
|
||||
assert name in _COMMAND_REGISTRY, f"registry missing {name}"
|
||||
|
||||
|
||||
def test_compat_content_discovery_surfaces_route_explanation_with_vector_secondary(tmp_path):
|
||||
"""Compat mode surfaces paper_fts primary + vector secondary when available."""
|
||||
from paperforge.embedding import get_embed_status
|
||||
|
||||
_make_paper_only_db(tmp_path)
|
||||
result = route_gateway(tmp_path, "content-discovery", "Johnson", json_mode=True)
|
||||
|
||||
assert result.ok
|
||||
route_exp = result.data["route_explanation"]
|
||||
assert route_exp["primary_arm"] == "paper_fts"
|
||||
assert route_exp.get("compatibility_mode") is True
|
||||
# Vector may or may not be available; verify the response shape either way
|
||||
if "secondary_arm" in route_exp:
|
||||
assert route_exp["secondary_arm"] == "vector_retrieve"
|
||||
# When vector is available, vector_results should be populated
|
||||
if result.data.get("vector_results") is not None:
|
||||
assert isinstance(result.data["vector_results"], list)
|
||||
else:
|
||||
# vector_results is None when vector is unavailable — clean fallback
|
||||
assert result.data.get("vector_results") is None
|
||||
assert len(result.data["results"]) > 0
|
||||
|
|
|
|||
|
|
@ -137,8 +137,10 @@ def test_build_object_units_from_role_index():
|
|||
units = build_object_units(tree=tree, structured_blocks=blocks, role_index=role_index)
|
||||
assert len(units) >= 1
|
||||
assert units[0]["unit_id"].startswith("P010:object:")
|
||||
assert "Figure 1: Results" in units[0]["unit_text"]
|
||||
assert units[0]["object_role"] == "figure_captions"
|
||||
assert units[0]["object_kind"] == "figure"
|
||||
assert units[0]["object_label"] == "Figure 1"
|
||||
assert units[0]["caption_text"] == "Figure 1: Results"
|
||||
assert "As shown in Figure 1." in units[0]["nearby_body_text"]
|
||||
|
||||
|
||||
def test_build_object_units_empty_role_index():
|
||||
|
|
@ -186,7 +188,7 @@ def test_build_paper_manifest():
|
|||
def test_fts_body_units_table_creation(tmp_path):
|
||||
db = tmp_path / "test.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.execute("PRAGMA journal_mode=WAL;")
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute(CREATE_BODY_UNITS)
|
||||
conn.execute(CREATE_BODY_UNITS_FTS)
|
||||
conn.commit()
|
||||
|
|
@ -224,9 +226,11 @@ def test_fts_body_units_table_creation(tmp_path):
|
|||
).fetchall()
|
||||
assert len(results) == 1
|
||||
assert results[0][0] == "P001:body:sec:1:1-b1:1-b2"
|
||||
# Verify unit_kind was set via DEFAULT
|
||||
row = conn.execute("SELECT unit_kind FROM body_units WHERE unit_id = ?", ("P001:body:sec:1:1-b1:1-b2",)).fetchone()
|
||||
assert row["unit_kind"] == "body"
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_object_units_table_creation(tmp_path):
|
||||
db = tmp_path / "test.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
|
|
@ -234,22 +238,27 @@ def test_object_units_table_creation(tmp_path):
|
|||
conn.execute(CREATE_OBJECT_UNITS)
|
||||
conn.commit()
|
||||
conn.execute(
|
||||
"""INSERT INTO object_units (unit_id, paper_id, section_path, unit_text,
|
||||
object_role, page_span_json, block_span_json)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)""",
|
||||
"""INSERT INTO object_units (unit_id, paper_id, section_path,
|
||||
object_kind, object_label, caption_text, nearby_body_text,
|
||||
page_span_json, block_span_json)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)""",
|
||||
(
|
||||
"P001:object:sec:1:1-f1:1-f1",
|
||||
"P001",
|
||||
"Figures",
|
||||
"figure",
|
||||
"Figure 1",
|
||||
"Figure 1: Results",
|
||||
"figure_captions",
|
||||
"As shown in Figure 1.",
|
||||
json.dumps([4, 4]),
|
||||
json.dumps([[4, "f1"]]),
|
||||
),
|
||||
)
|
||||
row = conn.execute("SELECT unit_id, object_role FROM object_units WHERE unit_id = ?", ("P001:object:sec:1:1-f1:1-f1",)).fetchone()
|
||||
row = conn.execute("SELECT unit_id, object_kind, object_label, caption_text FROM object_units WHERE unit_id = ?", ("P001:object:sec:1:1-f1:1-f1",)).fetchone()
|
||||
assert row is not None
|
||||
assert row["object_role"] == "figure_captions"
|
||||
assert row["object_kind"] == "figure"
|
||||
assert row["object_label"] == "Figure 1"
|
||||
assert row["caption_text"] == "Figure 1: Results"
|
||||
conn.close()
|
||||
|
||||
|
||||
|
|
@ -268,3 +277,33 @@ def test_ensure_schema_includes_new_tables(tmp_path):
|
|||
fts_tables = [r[0] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE '%_fts%'").fetchall()]
|
||||
assert "body_units_fts" in fts_tables
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_body_units_schema_includes_unit_kind(tmp_path):
|
||||
db = tmp_path / "test_schema_body_units.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA journal_mode=WAL;")
|
||||
ensure_schema(conn)
|
||||
|
||||
cols = {
|
||||
r[1] for r in conn.execute("PRAGMA table_info(body_units)").fetchall()
|
||||
}
|
||||
assert "unit_kind" in cols, "body_units missing unit_kind column"
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_object_units_schema_includes_new_columns(tmp_path):
|
||||
db = tmp_path / "test_schema_obj_units.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA journal_mode=WAL;")
|
||||
ensure_schema(conn)
|
||||
|
||||
cols = {
|
||||
r[1] for r in conn.execute("PRAGMA table_info(object_units)").fetchall()
|
||||
}
|
||||
for col in ("object_kind", "object_label", "caption_text", "nearby_body_text"):
|
||||
assert col in cols, f"object_units missing {col} column"
|
||||
assert "object_role" not in cols, "object_units should not have legacy object_role column"
|
||||
conn.close()
|
||||
|
|
|
|||
Loading…
Reference in a new issue