feat(bib): FR jump links land on the passage — text-fragment highlight, page→¶ upgrade, quote tie-break (refs #636)
This commit is contained in:
@@ -13,6 +13,10 @@ any FR citation:
|
||||
- ``"91 FR 44218 ¶3"`` → 3rd paragraph on that page (``#p-N``)
|
||||
- ``"p-3600"`` → validated raw paragraph anchor
|
||||
- a quoted passage → the paragraph containing it
|
||||
- ``highlight=True`` → adds a ``#p-N:~:text=…`` scroll-to-text
|
||||
fragment (first sentence) so the browser scrolls *and* highlights the
|
||||
passage; page-only cites upgrade to the first paragraph starting on
|
||||
that page. The notebook/chat default; ``place()`` stores plain anchors.
|
||||
|
||||
Transmutation runs both directions (:func:`page_of`,
|
||||
:func:`paragraphs_of`), :func:`md_link` renders markdown for
|
||||
@@ -50,6 +54,32 @@ _FR_CITE_RE = re.compile(
|
||||
)
|
||||
|
||||
_MIN_QUOTE_LEN = 15
|
||||
_SENTENCE_END = re.compile(r"(?<=[.;:!?])\s+")
|
||||
|
||||
|
||||
def text_fragment(text: str, max_chars: int = 80) -> str:
|
||||
"""URL fragment-directive body for ``#:~:text=…`` — the paragraph's
|
||||
first sentence, clipped at a word boundary to ``max_chars``.
|
||||
|
||||
Percent-encodes per the Text Fragments spec: everything but
|
||||
unreserved chars, and ``-`` as ``%2D`` (a bare dash is the
|
||||
prefix/suffix separator). Empty input → ``""``.
|
||||
"""
|
||||
from urllib.parse import quote
|
||||
|
||||
text = " ".join(text.split())
|
||||
if not text:
|
||||
return ""
|
||||
first = _SENTENCE_END.split(text, maxsplit=1)[0]
|
||||
if len(first) > max_chars:
|
||||
cut = first[:max_chars].rsplit(" ", 1)[0] or first[:max_chars]
|
||||
first = cut.rstrip(" ,;:")
|
||||
return quote(first, safe="").replace("-", "%2D")
|
||||
|
||||
|
||||
def _with_fragment(url: str, text: str) -> str:
|
||||
frag = text_fragment(text)
|
||||
return f"{url}:~:text={frag}" if frag else url
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -264,9 +294,12 @@ def _anchor_row(con: Any, item_key: str, p_id: int) -> Any:
|
||||
).fetchone()
|
||||
|
||||
|
||||
def _para_link(doc: Any, row: Any) -> JumpLink:
|
||||
def _para_link(doc: Any, row: Any, *, highlight: bool = False) -> JumpLink:
|
||||
url = f"{doc['html_url']}#p-{row['p_id']}"
|
||||
if highlight:
|
||||
url = _with_fragment(url, row["text"])
|
||||
return JumpLink(
|
||||
url=f"{doc['html_url']}#p-{row['p_id']}",
|
||||
url=url,
|
||||
item_key=doc["item_key"],
|
||||
page=row["page"],
|
||||
p_id=row["p_id"],
|
||||
@@ -275,7 +308,9 @@ def _para_link(doc: Any, row: Any) -> JumpLink:
|
||||
)
|
||||
|
||||
|
||||
def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
def resolve(
|
||||
ref: str, *, store: Store, item_key: str = "", highlight: bool = False
|
||||
) -> JumpLink:
|
||||
"""Resolve any FR reference to a deep jump link.
|
||||
|
||||
Accepted forms, in precedence order: raw anchor (``p-3600``, needs
|
||||
@@ -283,7 +318,13 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
(``91 FR 44218`` → page link; ``91 FR 44218 ¶3``/``para 3``/``p.3``
|
||||
→ that page's 3rd paragraph), else a quoted passage (≥ 15 chars)
|
||||
searched over the stored paragraph text — exactly one match
|
||||
required.
|
||||
required (ties broken toward the paragraph that *starts* with the
|
||||
quote, then the shortest).
|
||||
|
||||
``highlight=True`` appends a ``:~:text=`` scroll-to-text fragment
|
||||
(first sentence) to paragraph links so browsers scroll *and*
|
||||
highlight the passage, and upgrades a page-only cite to the first
|
||||
paragraph starting on that page when the page has anchors.
|
||||
"""
|
||||
ref = " ".join(ref.split())
|
||||
con = store._con() # noqa: SLF001
|
||||
@@ -304,7 +345,7 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
row = _anchor_row(con, doc["item_key"], p_id)
|
||||
if row is None:
|
||||
raise ValueError(f"{doc['item_key']}: no anchor {ref} (p-{p_id})")
|
||||
return _para_link(doc, row)
|
||||
return _para_link(doc, row, highlight=highlight)
|
||||
|
||||
m = _FR_CITE_RE.match(ref)
|
||||
if m:
|
||||
@@ -320,6 +361,14 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
f"{doc['start_page']}-{doc['end_page']}"
|
||||
)
|
||||
if m.group(3) is None:
|
||||
if highlight:
|
||||
first = con.execute(
|
||||
"SELECT p_id, page, ordinal, text FROM fr_anchors "
|
||||
"WHERE item_key = ? AND page = ? ORDER BY ordinal LIMIT 1",
|
||||
(doc["item_key"], page),
|
||||
).fetchone()
|
||||
if first is not None:
|
||||
return _para_link(doc, first, highlight=True)
|
||||
return JumpLink(
|
||||
url=f"{doc['html_url']}#page-{page}",
|
||||
item_key=doc["item_key"],
|
||||
@@ -341,7 +390,7 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
f"{ref}: page {page} has {len(rows)} paragraph(s), "
|
||||
f"¶{ordinal} does not exist"
|
||||
)
|
||||
return _para_link(doc, rows[ordinal - 1])
|
||||
return _para_link(doc, rows[ordinal - 1], highlight=highlight)
|
||||
|
||||
quote = _norm(ref)
|
||||
if len(quote) < _MIN_QUOTE_LEN:
|
||||
@@ -360,6 +409,17 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
hits = con.execute(sql, params).fetchall()
|
||||
if not hits:
|
||||
raise ValueError(f"{ref!r}: no paragraph contains this quote")
|
||||
if len(hits) > 1:
|
||||
lower = quote.lower()
|
||||
starts = [h for h in hits if h["text"].lower().startswith(lower)]
|
||||
if len(starts) == 1:
|
||||
hits = starts
|
||||
else:
|
||||
pool = starts or hits
|
||||
shortest = min(len(h["text"]) for h in pool)
|
||||
tight = [h for h in pool if len(h["text"]) == shortest]
|
||||
if len(tight) == 1:
|
||||
hits = tight
|
||||
if len(hits) > 1:
|
||||
where = ", ".join(f"{h['item_key']} p-{h['p_id']}" for h in hits[:5])
|
||||
raise ValueError(
|
||||
@@ -368,7 +428,7 @@ def resolve(ref: str, *, store: Store, item_key: str = "") -> JumpLink:
|
||||
)
|
||||
hit = hits[0]
|
||||
doc = _doc_for_key(store, hit["item_key"])
|
||||
return _para_link(doc, hit)
|
||||
return _para_link(doc, hit, highlight=highlight)
|
||||
|
||||
|
||||
# ── Transmutations + rendering ──────────────────────────────────────
|
||||
@@ -397,9 +457,17 @@ def paragraphs_of(ref: str, *, store: Store, item_key: str = "") -> list[Anchor]
|
||||
]
|
||||
|
||||
|
||||
def md_link(ref: str, *, store: Store, item_key: str = "", text: str = "") -> str:
|
||||
"""Markdown ``[text](url)`` for a resolved FR reference."""
|
||||
link = resolve(ref, store=store, item_key=item_key)
|
||||
def md_link(
|
||||
ref: str,
|
||||
*,
|
||||
store: Store,
|
||||
item_key: str = "",
|
||||
text: str = "",
|
||||
highlight: bool = True,
|
||||
) -> str:
|
||||
"""Markdown ``[text](url)`` for a resolved FR reference — highlighted
|
||||
(scroll-to-text) by default so notebook links land on the passage."""
|
||||
link = resolve(ref, store=store, item_key=item_key, highlight=highlight)
|
||||
return f"[{text or ref}]({link.url})"
|
||||
|
||||
|
||||
|
||||
@@ -89,7 +89,9 @@ class Pincite(BaseModel):
|
||||
try:
|
||||
from bib import frlink
|
||||
|
||||
return frlink.resolve(self.locator, store=store, item_key=self.item_key).url
|
||||
return frlink.resolve(
|
||||
self.locator, store=store, item_key=self.item_key, highlight=True
|
||||
).url
|
||||
except Exception: # noqa: BLE001 — degrade to no-URL by contract
|
||||
return ""
|
||||
|
||||
|
||||
@@ -185,14 +185,14 @@ class TestResolve:
|
||||
assert link.page == 101
|
||||
s.close()
|
||||
|
||||
def test_quote_multi_hit_raises(self) -> None:
|
||||
def test_quote_multi_hit_prefers_shortest_start_match(self) -> None:
|
||||
dup_html = FIXTURE_HTML.replace(
|
||||
"</body>",
|
||||
'<p id="p-9" data-page="101">Only para on 101, repeated.</p></body>',
|
||||
)
|
||||
s, _key = _grabbed_store(dup_html)
|
||||
with pytest.raises(ValueError, match="matches 2"):
|
||||
frlink.resolve("Only para on 101", store=s)
|
||||
# both start with the quote → the tighter paragraph wins
|
||||
assert frlink.resolve("Only para on 101", store=s).p_id == 3
|
||||
s.close()
|
||||
|
||||
def test_quote_no_hit_raises(self) -> None:
|
||||
@@ -230,13 +230,15 @@ class TestTransmutations:
|
||||
class TestMdLink:
|
||||
def test_default_text_is_ref(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
md = frlink.md_link("91 FR 100 ¶2", store=s)
|
||||
md = frlink.md_link("91 FR 100 ¶2", store=s, highlight=False)
|
||||
assert md == "[91 FR 100 ¶2](https://example.test/doc#p-2)"
|
||||
s.close()
|
||||
|
||||
def test_custom_text(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
md = frlink.md_link("91 FR 100", store=s, text="the CF discussion")
|
||||
md = frlink.md_link(
|
||||
"91 FR 100", store=s, text="the CF discussion", highlight=False
|
||||
)
|
||||
assert md == "[the CF discussion](https://example.test/doc#page-100)"
|
||||
s.close()
|
||||
|
||||
@@ -268,3 +270,93 @@ class TestPlace:
|
||||
assert rows[0]["label"] == "91 FR 100"
|
||||
assert rows[0]["p_id"] is None
|
||||
s.close()
|
||||
|
||||
|
||||
# ── text fragments + highlight ──────────────────────────────────────
|
||||
|
||||
|
||||
class TestTextFragment:
|
||||
def test_first_sentence_encoded(self) -> None:
|
||||
frag = frlink.text_fragment(
|
||||
"Under this proposal, the new G-codes apply. Second sentence."
|
||||
)
|
||||
assert frag == "Under%20this%20proposal%2C%20the%20new%20G%2Dcodes%20apply."
|
||||
|
||||
def test_long_sentence_cut_at_word_boundary(self) -> None:
|
||||
text = "word " * 40
|
||||
frag = frlink.text_fragment(text.strip(), max_chars=22)
|
||||
assert frag == "word%20word%20word%20word"
|
||||
|
||||
def test_empty(self) -> None:
|
||||
assert frlink.text_fragment(" ") == ""
|
||||
|
||||
|
||||
class TestHighlight:
|
||||
def test_paragraph_ref_gets_fragment(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
link = frlink.resolve("91 FR 100 ¶2", store=s, highlight=True)
|
||||
assert link.url == (
|
||||
"https://example.test/doc#p-2:~:text=Second%20para%20with%20markup%20on%20100."
|
||||
)
|
||||
assert link.p_id == 2
|
||||
s.close()
|
||||
|
||||
def test_page_cite_upgrades_to_first_paragraph_on_page(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
link = frlink.resolve("91 FR 101", store=s, highlight=True)
|
||||
assert link.p_id == 3
|
||||
assert link.url.startswith("https://example.test/doc#p-3:~:text=Only%20para")
|
||||
s.close()
|
||||
|
||||
def test_page_cite_without_highlight_unchanged(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
link = frlink.resolve("91 FR 101", store=s)
|
||||
assert link.url == "https://example.test/doc#page-101"
|
||||
assert link.p_id is None
|
||||
s.close()
|
||||
|
||||
def test_page_without_anchors_stays_page_link_even_when_highlighting(
|
||||
self,
|
||||
) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
s._con().execute("DELETE FROM fr_anchors WHERE page = 101")
|
||||
link = frlink.resolve("91 FR 101", store=s, highlight=True)
|
||||
assert link.url == "https://example.test/doc#page-101"
|
||||
s.close()
|
||||
|
||||
def test_md_link_highlights_by_default(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
md = frlink.md_link("91 FR 100 ¶1", store=s)
|
||||
assert ":~:text=" in md
|
||||
assert md.startswith("[91 FR 100 ¶1](")
|
||||
s.close()
|
||||
|
||||
def test_place_never_records_fragment(self) -> None:
|
||||
s, _key = _grabbed_store()
|
||||
link = frlink.place(s, "91 FR 100 ¶1", label="x")
|
||||
assert ":~:text=" not in link.url
|
||||
(row,) = s._con().execute("SELECT url FROM fr_links").fetchall()
|
||||
assert ":~:text=" not in row["url"]
|
||||
s.close()
|
||||
|
||||
|
||||
class TestQuoteTieBreak:
|
||||
def test_prefers_paragraph_starting_with_quote(self) -> None:
|
||||
html = """
|
||||
<p id="p-1" data-page="100">Shared quote text appears here first.</p>
|
||||
<p id="p-2" data-page="100">Before it, shared quote text appears here again.</p>
|
||||
"""
|
||||
s, _key = _grabbed_store(html)
|
||||
link = frlink.resolve("shared quote text appears here", store=s)
|
||||
assert link.p_id == 1
|
||||
s.close()
|
||||
|
||||
def test_still_raises_when_truly_ambiguous(self) -> None:
|
||||
html = """
|
||||
<p id="p-1" data-page="100">Shared quote text appears here first.</p>
|
||||
<p id="p-2" data-page="100">Shared quote text appears here again.</p>
|
||||
"""
|
||||
s, _key = _grabbed_store(html)
|
||||
with pytest.raises(ValueError, match="matches 2 paragraphs"):
|
||||
frlink.resolve("shared quote text appears here", store=s)
|
||||
s.close()
|
||||
|
||||
@@ -535,7 +535,9 @@ class TestJumpUrl:
|
||||
locator="91 FR 100 ¶2",
|
||||
locator_type="fr_para",
|
||||
)
|
||||
assert p.jump_url(s) == "https://example.test/doc#p-2"
|
||||
assert p.jump_url(s) == (
|
||||
"https://example.test/doc#p-2:~:text=Second%20para%20with%20markup%20on%20100."
|
||||
)
|
||||
s.close()
|
||||
|
||||
def test_jump_url_resolves_plain_fr(self) -> None:
|
||||
@@ -543,7 +545,8 @@ class TestJumpUrl:
|
||||
p = Pincite(
|
||||
fn_path="mod.fn", item_key=key, locator="91 FR 100", locator_type="fr"
|
||||
)
|
||||
assert p.jump_url(s) == "https://example.test/doc#page-100"
|
||||
# page cites upgrade to the first paragraph starting on that page
|
||||
assert p.jump_url(s).startswith("https://example.test/doc#p-1:~:text=First")
|
||||
s.close()
|
||||
|
||||
def test_jump_url_empty_when_unresolvable(self) -> None:
|
||||
@@ -580,7 +583,7 @@ class TestGraphJumpUrl:
|
||||
)
|
||||
g = build_citation_graph(s)
|
||||
edge = next(e for e in g.edges if e.source == "mod.fn")
|
||||
assert edge.jump_url == "https://example.test/doc#p-2"
|
||||
assert edge.jump_url.startswith("https://example.test/doc#p-2:~:text=")
|
||||
s.close()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user