Uh oh!
There was an error while loading. Please reload this page.
V111 PDF library tables + #511/#512/#515 follow-ups - #516
Merged
Conversation
V111 adds four catalogue-side tables backing `phoenix_kit_catalogue`'s PDF library, layered on top of `phoenix_kit_files`: - `phoenix_kit_cat_pdfs` — per-upload row. `file_uuid` FK to `phoenix_kit_files(uuid)` ON DELETE RESTRICT (catalogue manages the file lifecycle; core prune can't remove a file referenced by a live catalogue row). Soft-delete via `status` sentinel. - `phoenix_kit_cat_pdf_extractions` — keyed by `file_uuid` PK. Worker state machine + page count + error message. Cascades on file delete. - `phoenix_kit_cat_pdf_page_contents` — content-addressed page text dedup cache (PK on SHA-256 of normalized text). GIN trigram index on `text` for the per-item search. - `phoenix_kit_cat_pdf_pages` — composite-PK join `(file_uuid, page_number)` → `content_hash` FK to the dedup cache. Cascade on file, restrict on content. Enables `pg_trgm`. `@current_version` bumped 110 → 111. Also: `<.file_upload>` (`PhoenixKitWeb.Components.Core.FileUpload`) now labels the in-flight progress bar `Uploading…` rather than the bare percentage. `entry.progress` is always client→server upload progress (Phoenix LV convention), so the wording is universally accurate. `MediaBrowser` and any future consumers inherit the clarified label.
Triage of CLAUDE_REVIEW.md against current code: every actionable item was addressed in the reviewer's own post-merge follow-up commit (`Runner.runner_opts/1` + prefix-forwarding tests + `## Return contract` moduledoc + `@version 1.7.105`). FOLLOW_UP.md records the audit so the PR folder isn't ambiguously "untriaged".
Triage of CLAUDE_REVIEW.md against current code: all six findings (5 NITPICKs + 1 LOW) addressed pre-existing — `:reorder_scope` camelCase round-trip doc, `:on_reorder` asymmetry doc, `data-id` unconditionally emitted, V108 sort stability covered by downstream catalogue's `[asc: :position, asc: :name]` ordering. FOLLOW_UP.md records the audit.
…LOW_UP.md Closes the one remaining MEDIUM finding from CLAUDE_REVIEW.md (BeamLabEU#2 — hardcoded `/common/` tenant in M365 OAuth URLs that broke single- tenant operators with AADSTS50194). Two-part fix: 1. New `interpolate_url/3` private helper in `OAuth` substitutes `{key}` placeholders with values from the integration's data (string-keyed JSONB), falling back to `oauth_config[:url_defaults]`. Wired into `authorization_url/5`, `exchange_code/4`, and `refresh_access_token/2`. URLs without `{` pass through unchanged (zero impact on Google / OpenRouter / Mistral / DeepSeek). 2. Microsoft provider's `auth_url` / `token_url` now use `{tenant_id}`; new `url_defaults: %{"tenant_id" => "common"}` preserves multi-tenant default behavior; new optional `tenant_id` setup field with help text covering the GUID / `consumers` / `organizations` options. Instructions panel note rewritten — operators fill in the form field rather than manually editing URLs. Three pinning tests added to OAuth test suite (interpolation with a GUID; fallback to url_defaults; backwards-compat pass-through for URLs without placeholders). All 21 OAuth tests pass. The other nine reviewer findings (1 MEDIUM doc, 2 LOW doc/typo, 6 NITPICKs) were addressed pre-existing or reviewer-acknowledged as non-blocking — see FOLLOW_UP.md.
…follow-up) Closes NITPICK BeamLabEU#10 from CLAUDE_REVIEW.md on PR BeamLabEU#511. The lenient `:ok` return on the missing-state branch of verify_oauth_state/2 dated from an older flow that didn't always save state before redirect; that flow is gone (every connect_oauth event now calls save_oauth_state/2 first at integration_form.ex:227). A missing state at callback time now implies either someone bypassed connect_oauth or the row was mutated between authorize and callback — both are CSRF-relevant. Returns {:error, :state_mismatch} on that branch with an updated comment explaining why. No tests exercised the lenient branch (verified via grep). The {:error, :state_mismatch} return shape is already handled by the caller's existing mismatch case. FOLLOW_UP.md updated to reflect the closure (moves from Skipped to Fixed Batch 1; NITPICK BeamLabEU#6 stays Open pending Max's decision).
… follow-up) Closes NITPICK BeamLabEU#6 from CLAUDE_REVIEW.md on PR BeamLabEU#511, but as a code fix rather than the doc-note the reviewer suggested. The picker was rendering provider name in place of `conn.name` whenever `conn.name == "default"` (a pre-BeamLabEU#511 carryover from when `default` was system-privileged). PR BeamLabEU#511's own moduledoc explicitly says "Names are pure user-chosen labels with no system semantics" — the picker contradicting that is the actual bug. Now always renders `conn.name` verbatim. Provider badge is also unconditional (drop the `conn.name != "default"` guard) so users always see which provider they're picking regardless of how the connection is named. Only call site of the substitution; no other surface depended on it. Two test references to `name == "default"` are persistence assertions, not picker rendering — unaffected. FOLLOW_UP.md updated: NITPICK BeamLabEU#6 moves to Fixed (Batch 1); Open is now None.
ddon added a commit
that referenced
this pull request
May 8, 2026
Eight post-merge reviews covering V111 PDF tables, the DB-module extraction, MediaBrowser modal + LV login return_to, external-plugin admin permissions, dashboard sidebar gettext, live Hex.pm known-packages fetch, the publishing routing-strategy shim, and the LanguageSwitcher per_translation_urls attr (plus the bundled DnD work). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon added a commit
that referenced
this pull request
May 8, 2026
…525 Code/doc fixes addressing one finding per PR (or several where trivial). Each closes a NITPICK or IMPROVEMENT-LOW from the matching CLAUDE_REVIEW.md; design-level / breaking / risky items deferred per the FOLLOW_UP.md "Skipped" sections. - #516: Drop dead `String.to_atom` fallback in OAuth interpolate_url - #518: Delete stray 0-byte pages_html.ex - #519: Fix stale `viewer={true}` template comment + login_path trailing-slash self-loop guard - #521: Resolution-order doc on permission_key_for_admin_view/1 - #522: Hot-reload safety pitfall in per-module-i18n.md - #523: KnownPackages — max-pages cap, ensure_table race comment, Logger-levels operational signals in moduledoc - #524: __mix_recompile__?/0 note next to apply/3 explanation - #525: LanguageSwitcher attr doc atom/string keys + DRY resolve_url per-language + JS sortable:flash defensive status check Plus FOLLOW_UP.md per PR enumerating closed vs deferred items. PR #525's FOLLOW_UP also captures the bundled DnD audit trail (table_default drag-handle scoping, sortable:flash, TR cell-width preservation) absent from the original PR body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon added a commit
that referenced
this pull request
May 8, 2026
Three-day window of accumulated work since 1.7.105 (2026-05-05): PRs #516, #518, #519, #521, #522, #523, #524, #525, plus the review-doc suite and post-merge triage closing nitpicks across all eight. Headline changes — V111 PDF library tables, DB module extracted to phoenix_kit_db, MediaBrowser modal viewer, sidebar gettext API, live Hex.pm catalog, publishing routing-strategy shim closing the /:locale/<literal>/... host-route shadowing bug, LanguageSwitcher :per_translation_urls, and bundled DnD improvements (drag-handle scoping, sortable:flash, TR cell-width preservation). All changes are strictly additive / non-breaking; one transitional extraction (DB → phoenix_kit_db) requires the paired Hex package once it ships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds V111 — four new tables backing
phoenix_kit_catalogue's PDFlibrary (per-upload row + per-file extraction state + content-addressed
page-text dedup + composite-PK page join), enables
pg_trgm. Plus aone-line clarification to
<.file_upload>'s progress label(
Uploading…instead of bare percentage) so consumers —MediaBrowserand the new catalogue PDF library — surface unambiguous client→server
upload state. Plus PR follow-ups for #511 / #512 / #515 with all
remaining live findings closed.
Pairs with
BeamLabEU/phoenix_kit_catalogue#24(the consumer of V111).PR follow-up(s)
PR Tighten Integrations to strict-UUID + V107 endpoint UNIQUE index + V106 follow-up + provider registry expansion #511 (
9a6b9fe9review-only stub →8a5a393b+c6c4a89e+0447564cactual fixes) — 3 of 4 doc/code fixes from the originalCLAUDE_REVIEW had already landed pre-existing; this batch closes
the remaining live items:
/common/tenant;single-tenant operators got AADSTS50194. Added
tenant_idsetupfield with
commondefault + genericinterpolate_url/3helperin
OAuththat substitutes{key}placeholders fromintegration_data(zero impact on Google / OpenRouter / Mistral/ DeepSeek). Three pinning tests added.
verify_oauth_state/2now returns{:error, :state_mismatch}on missing stored state (was lenient:okfrom a pre-PR-511 flow that's gone).
IntegrationPickerremoved theconn.name == \"default\"substitution that contradicted PR Tighten Integrations to strict-UUID + V107 endpoint UNIQUE index + V106 follow-up + provider registry expansion #511's ownmoduledoc ("Names are pure user-chosen labels with no system
semantics"). Always renders the user-chosen name + provider
badge.
dev_docs/pull_requests/2026/511-strict-uuid-integrations-v107/.PR Add V108 + drag-and-drop core + PR #506 review follow-up #512 (
09ee467dstub) — all 6 reviewer findings (5 NITPICKsthe audit;
Open: None.PR Add ensure_current/2 helper + V110 language column on doc templates #515 (
9a6b9fe9stub) — reviewer's own post-merge follow-upcommit landed pre-existing (
Runner.runner_opts/1+ tests +Return contract docs +
@version 1.7.105). FOLLOW_UP.md recordsthe audit;
Open: None.V111 — PDF library tables (
31bd0cb2)Four tables, all idempotent
create_if_not_existspatterns matchingrecent V108–V110 style:
phoenix_kit_cat_pdfs— thin per-upload row.file_uuidFK tophoenix_kit_files(uuid)ON DELETE RESTRICT (catalogue managesfile lifecycle; core prune can't remove a file referenced by a
live catalogue row). Soft-delete via
statussentinel(
active/trashed) +trashed_at. Two uploads of identicalcontent (different filenames) → two rows sharing one
phoenix_kit_filesrow + one extraction.
phoenix_kit_cat_pdf_extractions— keyed byfile_uuidPK.Worker state machine +
page_count+extracted_at+error_message. Cascades on file hard delete.phoenix_kit_cat_pdf_page_contents— content-addressed dedupcache. PK on
content_hash(SHA-256 of normalized page text).GIN trigram index lives here so the search index doesn't grow
with cross-PDF duplication.
phoenix_kit_cat_pdf_pages— composite PK(file_uuid, page_number)+content_hashFK to the dedup cache.Enables
pg_trgmextension.@current_versionbumped 110 → 111.Doc-block in
postgres.exupdated.<.file_upload>\"Uploading…\"progress label (31bd0cb2)One-line tweak: the entry-progress
<span>inPhoenixKitWeb.Components.Core.FileUpload'sfull_upload/1variantnow reads
Uploading… {entry.progress}%instead of the barepercentage.
entry.progressis always client→server upload progress(Phoenix LV convention), so the wording is universally accurate.
MediaBrowser(the only existing consumer) and the new cataloguePDF library uploads inherit the clarified label.
Verification
V111 verified to apply cleanly via
PhoenixKit.Migration.ensure_current/2on the catalogue's parent test DB (the
Applying PhoenixKit V110→V111log line in the boot output).
Test plan
mix precommitclean end-to-end (running phx.server on theshared dev DB blocks an ad-hoc full-suite run from this branch
this session)
up/1('111') anddown/1(
'110') per the workspace's marker-comment review ruleOpen
None across the three follow-ups. The
integration_pickerdivergencequestion (NIT #6) was resolved in code rather than docs after a
discussion: the substitution itself was the bug, not the missing
doc note.