Skip to content

REVIEW: Citation display helpers (#520) - #521

Open
rejojer wants to merge 3 commits into
review/base-9a8dd66from
feat/citation-helpers
Open

rejojer wants to merge 3 commits into
review/base-9a8dd66from
feat/citation-helpers

Conversation

@rejojer

@rejojer rejojer commented Sep 19, 2026

Copy link
Copy Markdown
Member

Review-only PR for #520. Do not merge.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review Completed 2026-09-19T13:40:25.457783Z ac3efcb PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@rejojer
rejojer force-pushed the feat/citation-helpers branch 2 times, most recently from d6a71dd to d70e771 Compare September 20, 2026 16:32
…s, folder paths

BREAKING: resolve_citations(), released in 0.2.17, is renamed
get_citations(); same arguments, same list. resolve_citations() now returns
{'answer', 'citations'}: the answer with each citation tag replaced by
[[i]](#pageindex-citation-0i), and the get_citations() entries led by
'anchor' and 'index'. Numbers follow the tags as written, so a repeated
citation reuses its number and a block cited under the wrong page still
gets its link while its entry carries the block's real page. One shared
_citation_key() parses a matched tag for both the parser and the rewrite.

highlight_region(image, bbox, scale=1000) draws the cited region on a page
image (PIL.Image or bytes in, PIL.Image out). Pillow becomes a dependency.

get_page_image(doc_id, page) and get_document_image(doc_id, img_id) return
short-lived URLs. Cloud-only. They need the compute routes that return
presigned URLs; against an older server they raise PageIndexAPIError.

get_document_path, get_folder_path and get_folder_id translate between ids
and readable paths. Paths are built from list_folders() parent links,
because the server's `path` field exists only on /docs listing rows. A path
shared by two folders raises instead of picking one.
<cite doc= page=>quoted</cite> and <cite doc= page=></cite> are tag
shapes the parser accepts and the chat renderer matches, but the
rewrite replaced only the opening tag, so the display text kept a raw
</cite>: "[[1]](#pageindex-citation-01)quoted</cite>". The chat UI hides
that because its HTML pipeline drops a stray end tag; this text goes to
any host. _CITE_TAG_RE now takes an optional "text</cite>" tail and the
link keeps the text. A closing tag is consumed only together with the
citation it closes, so a tag left unresolved, or a </cite> in quoted
HTML, stays as written. The parser reads group 1 as before: 0 diffs on
30,000 random tag inputs, and 1 MB bodies scan in about 15 ms.

The Returns section said every block-level citation carries bbox,
block_type and text. get_citations() adds them only when the block
could be read, so a local document or a 403/404 block has none, and
c['bbox'] raised KeyError for a reader of this docstring alone.

The highlight test drew on a 1000 px square at scale 1000 and probed
one pixel on the corner of the transposed rectangle, so an x/y swap, an
ignored scale and a whole-page fill all passed. It now draws on a
500x2000 page and checks one pixel inside the region and one outside;
all three mutants fail it.
@rejojer
rejojer force-pushed the feat/citation-helpers branch from d70e771 to cbbf9f6 Compare September 20, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant