feature/image-columns: composed-doc data persistence, Google Docs multi-column helpers, unified Active/Trash tabs

[timujinne]

Bundles five commits accumulated on feature/image-columns:

1. Persist composed-document creation settings in Document.data — Composer writes a caller-supplied :data opt into the Document changeset; schema_to_file_map/1 exposes the data field. Lets the host app store the template/image selection ("recipe") that produced a document so it can be re-created later.

2. Page content width and column width helpers in google_docs_client.ex — content_width_pt/1 extracts page width minus margins; image_width_for_columns/2 computes per-column image width accounting for inter-column gaps. Covered by google_docs_client_width_test.exs.

3. Table-based image insertion helpers for multi-column slots — table_image_inserts/3 (Phase A: delete placeholder + insertTable) and fill_table_cells/3 (Phase B: zip cells with media, reverse for last-first insertion to avoid index drift, build insertInlineImage requests with proper objectSize). Covered by google_docs_client_table_test.exs.

4. Unify Active/Trash sub-tabs on Categories page with Documents style — replaces boolean categories_trash/types_trash with string status_mode assigns and switch_status event matching DocumentsLive. Extracts status_subtabs/1 shared by Categories and Types columns, rendered in the same border-b-2 / border-primary / border-error style as Documents. Auto-hides the sub-tab row when Trash is empty and the current mode is active. New trashed_categories_count / trashed_types_count assigns are populated by the reload helpers, reusing the loaded list as the count when viewing trash to avoid a second query.

5. Regenerate _phoenix_kit_sources.css with @source directive — auto-generated content brought in sync with css_sources/0 callback.

mix format and mix quality are clean for the touched files. DB-backed CategoriesLive tests not run locally (no PostgreSQL).

[timujinne]

Verdict

Comment — minor issues only, no blockers.

Stage 1 — Spec compliance

Commit 1: 1643a97 — Persist composed-document creation settings in Document.data

• composer.ex: reads :data from opts with %{} default, passes it into Document.changeset. Verified.
• documents.ex: schema_to_file_map/1 now includes :data. Verified.
• composed_document_test.exs: two new tests covering presence and absence of :data. Verified.
• No silent unrelated changes. PASS.

Commit 2: ee0b312 — Add page content width and column width helpers

• content_width_pt/1 and image_width_for_columns/2 added with docs. Verified.
• Private magnitude/1 helper added. Verified.
• Module attributes @image_gap_pt, @default_content_width_pt, @max_columns added. Verified.
• google_docs_client_width_test.exs covers normal, fallback, and clamp cases. Verified.
• No silent unrelated changes. PASS.

Commit 3: 5d13816 — Add table-based image insertion helpers for multi-column slots

• table_image_inserts/3 (Phase A) and fill_table_cells/3 (Phase B) added. Verified.
• google_docs_client_table_test.exs covers both phases. Verified.
• No silent unrelated changes. PASS.

Commit 4: 064c0d2 — Unify Active/Trash sub-tabs on Categories page with Documents style

• Boolean assigns replaced with string mode assigns. Verified.
• toggle_categories_trash/toggle_types_trash replaced by switch_status with phx-value-target and phx-value-mode. Verified.
• status_subtabs/1 extracted as a shared component. Verified.
• Auto-hide when trash is empty implemented via :if={@trashed_count > 0 or @status_mode == "trashed"}. Verified.
• trashed_categories_count/trashed_types_count tracked. Verified.
• No silent unrelated changes. PASS.

Commit 5: 91605f2 — Regenerate _phoenix_kit_sources.css

• Single-line change: empty line replaced with @source directive. Verified.
• Consistent with the "auto-generated" header comment. PASS.

Stage 2 — Code quality

[MINOR] Extra DB query on every reload_categories in active mode

File: lib/phoenix_kit_document_creator/web/categories_live.ex:637-640
When status_mode == "active" (the common case), reload_categories fires a second Taxonomy.list_categories(status: "deleted") query solely to count trashed items. Same pattern in reload_types at lines 669-672 — every reload in active mode doubles the query count.
Suggestion: introduce a Taxonomy.count_categories(status: "deleted") that does Repo.aggregate(query, :count) instead of loading full records to call length/1. Acceptable for now since category/type lists are small, but worth flagging before it becomes ingrained.

[MINOR] table_image_inserts/3 opts accept but ignore content_width_pt

File: lib/phoenix_kit_document_creator/google_docs_client.ex:925
The function takes opts (map), and the test passes %{columns: 2, content_width_pt: 468.0}, but content_width_pt is never used inside the function — it is only used by Phase B (fill_table_cells).
Suggestion: either document that content_width_pt is meaningful only for Phase B, or narrow Phase A's opts to %{columns: _} and drop the unused key from the test.

[MINOR] scale_height/3 returns integer via round/1; callers depend on * 1.0 to coerce to float

File: lib/phoenix_kit_document_creator/google_docs_client.ex:1099-1100
scale_height/3 calls round(target_width * src_height / src_width) which returns an integer, while the Google Docs API magnitude field expects a float. The * 1.0 multiplication in fill_table_cells coerces it back, so it works at runtime, but this is an implicit caller obligation.
Suggestion: drop round/1 (the API accepts fractional points) or use Float.round/2. Pre-existing code, so flagging as a note rather than a request.

[NITPICK] Integer ceiling via float path

File: lib/phoenix_kit_document_creator/google_docs_client.ex:929
(length(media) / cols) |> Float.ceil() |> trunc() routes integer math through a float for the ceil. Idiomatic integer ceil-div is div(length(media) + cols - 1, cols). Style preference only — current code is correct.

[NITPICK] Catch-all handle_event("switch_status", _params, socket) silently drops malformed events

File: lib/phoenix_kit_document_creator/web/categories_live.ex:86-88
If a switch_status event arrives with an unexpected target or mode, it's silently ignored. Fine for production robustness, but a Logger.warning("Unexpected switch_status params: #{inspect(params)}") would help catch template typos during development.

[NOTE] Trash tab now uses border-error instead of border-primary

File: lib/phoenix_kit_document_creator/web/categories_live.ex:479-497
Deliberate UX change to match DocumentsLive — flagged as intentional, not a finding.

Summary

• 0 critical, 0 major, 3 minor, 2 nitpick, 1 note
• All five commits do exactly what their messages claim with no silent side effects.
• Tests cover the new helpers well; LiveView DB-backed tests acknowledged as requiring PostgreSQL and not run locally.
• The duplicate count query in reload_categories/reload_types (minor) is the most actionable item but is not blocking given current list sizes.

Go/no-go: ship it. None of the minor/nitpick items block merge.

[timujinne]

Verdict (new commits since previous review)

Comment — no blockers. Two carryovers from the previous review have now been addressed in a follow-up commit; remaining items are minor and one nitpick.

Stage 1 — Spec compliance

Commit 182de63 — Wire columns through orchestrator with two-phase table insertion

• Column-aware build_image_batch_requests/3 added; arity-2 delegates to it with @default_content_width_pt (backward compatible).
• list_image_inserts dispatches on :columns: >= 2 → table_image_inserts (Phase 1), == 1 → inline_image_inserts_pt.
• inline_image_inserts_pt builds insertInlineImage requests with PT-based width.
• Two-phase orchestration in substitute_all_images: Phase 1 creates tables + inline slots; Phase 2 re-fetches the doc and fills table cells.
• normalize_columns handles integer, binary, and nil inputs; clamps to [1, @max_columns].
• Test coverage in google_docs_client_phase_test.exs covers columns=1 inline path, columns≥2 table path, mixed slots, cell extraction, and column normalization pass-through.

PASS.

Commit fc69fa1 — Show deletion metadata in trash view and handle Drive 404 on restore

• JSONB stamp on delete via stamp_deleted_data/2 (data || jsonb_build_object(...)); clear on restore via clear_deleted_data/1 (data - 'deleted').
• "Deleted" column appears only on the trash tab in both list and cards views for Documents and Templates.
• User-name resolution via batch PhoenixKit.Users.Auth.get_users_by_uuids/1.
• do_move_file/2 distinguishes HTTP 404 and returns {:error, :drive_file_not_found}; propagated cleanly through restore_document/2/restore_template/2 to the LiveView, which shows a yellow dismissible alert-warning.
• :drive_file_not_found added to errors.ex type union and message/1.
• Tests in errors_test.exs and drive_bound_actions_test.exs cover the error message, stamp on delete (with and without actor, and preserving sibling keys), clear on restore, and Drive 404 for both Document and Template.

PASS.

Commit 6944db2 — Address previous review carryovers

• COALESCE(data, '{}'::jsonb) added to both fragments of stamp_deleted_data/2 (Template + Document) and both fragments of clear_deleted_data/1. Handles legacy rows with NULL data.
• Restore tests for Document and Template now include a sibling "recipe" => "preserved" key and assert it survives the restore.
• build_deleted_by_names/1 short-circuits to %{} when the extracted UUID list is empty, avoiding an unnecessary call to Auth.get_users_by_uuids/1 on the active tab.

PASS.

Stage 2 — Code quality

[MINOR] "Take last K tables" assumption in do_fill_table_cells/4

File: lib/phoenix_kit_document_creator/google_docs_client.ex:1634
Enum.take(all_tables, -k) assumes that newly inserted tables are the last K tables in the document. If the template already contains tables (layout table, data table, etc.), pre-existing tables shift the mapping and the wrong tables may be filled.
Suggestion: capture the count of pre-existing tables before Phase 1 (from doc2) and slice from that offset; or match by start_index against the known insertion points. At minimum, document the "no pre-existing tables" constraint in a moduledoc / function-doc comment.

[NITPICK] Atom vs string key inconsistency in batch requests

File: lib/phoenix_kit_document_creator/google_docs_client.ex:1119-1145
insert_inline_image_request_pt/3 uses atom keys (:insertInlineImage); table_image_inserts/fill_table_cells use string keys ("insertTable", "insertInlineImage"). Both go through the same batch_update/2. Works due to the JSON encoder, but reading the code requires switching mental models. Recommend standardising on one (preferably string keys, matching Google Docs API conventions).

Status of previous-review carryovers

• COALESCE in stamp/clear: fixed in 6944db2.
• Restore-preserves-other-keys test: fixed in 6944db2.

Summary

• 0 critical, 0 major, 1 minor, 1 nitpick.
• All previous-review carryovers addressed.
• The remaining minor item (table-take assumption) is an edge case that doesn't affect templates with no pre-existing tables — fine to ship and revisit when needed.

Go/no-go: ship it.