Review: Port block parser from Python to Ruby

review-1599-2026-06-24 Review

review needs-refinement

Project

[x] story:consolidation label -- absorb pal-e-docs into Rails monolith
[x] story note verified -- story-paldocs-consolidation found in project-paldocs user-stories section
[x] arch:rails-api label -- Rails API component
[ ] arch note MISSING -- [SCOPE] Create architecture note arch-rails-api for the Rails API component. Search returned zero results.
[x] Forgejo issue -- https://forgejo.tail5b443a.ts.net/ldraney/paldocs/issues/78, state: open

[x] app/services/block_parser.rb (new) -- target directory app/services/ does not exist yet but is standard Rails convention. Will need to be created. OK.
[x] app/services/block_compiler.rb (new) -- same as above. OK.
[x] app/controllers/api/notes_controller.rb -- verified at HEAD (commit 311f4a4). Has create (line 85) and update (line 167). Neither currently calls any block parsing. Confirmed gap.
[x] Reference: pal-e-docs/src/pal_e_docs/blocks/sync.py (112 lines), parser.py (316 lines), compiler.py (148 lines) -- all exist and verified.
[ ] MISSING target: Api::NotesController#from_template (line 125) also creates notes with html_content but is not listed. [BODY] Add from_template to File Targets and AC.

OK -- issue filed on ldraney/paldocs, all file targets are in the paldocs repo. Reference files in pal-e-docs are read-only source for the port.

Upstream (done): Issue #57 (API namespace + Notes controller) -- merged as commit 311f4a4. Provides the controller to modify.
Upstream (done): Issue #52 (Embedding pipeline) -- merged as commit 7d63900. Provides EmbedBlockJob and Block#after_save :enqueue_embedding callback. When this ticket creates Block rows via ActiveRecord, embeddings will be enqueued automatically.
Upstream (done): Issue #49 (Schema ownership) -- blocks table owned by Rails.
Downstream: Issue #61 (Hybrid search) depends on embeddings working, which depends on blocks being created. This ticket unblocks #61.
No blockers: No in_progress items block this ticket. Sprint:2 items (#56, #22, #75) are independent.

[x] AC 1: BlockParser.call(html_content) returns block hashes -- testable, clear.
[x] AC 2: BlockCompiler.call(blocks) reconstructs HTML -- testable, clear.
[x] AC 3: Api::NotesController#create stores blocks -- testable, clear.
[x] AC 4: Api::NotesController#update re-parses on content change -- testable, clear. Controller already tracks content_changed boolean.
[ ] AC 5: "Postgres NOTIFY fires on new block rows (existing trigger)" -- MISLEADING. The existing Postgres trigger (blocks_embedding_trigger from pal-e-docs Alembic migration) does fire pg_notify on INSERT, but the Rails embedding path uses ActiveRecord after_save callback on Block model to enqueue EmbedBlockJob via Solid Queue. Both paths coexist. [BODY] Reword AC 5: "Block rows created via ActiveRecord trigger after_save embedding enqueue (existing Block model callback)" or split into two: one for the Rails path (Solid Queue) and one noting NOTIFY coexistence.
[x] AC 6: Embedding worker picks up blocks -- will work automatically via existing Block#after_save :enqueue_embedding callback.
[ ] MISSING AC: from_template action also creates notes with html_content and needs block parsing. [BODY] Add AC: "Api::NotesController#from_template stores Block rows via parser".

sync.py recompile pattern: The Python parse_and_store_blocks() calls recompile() which updates a compiled_pages table and resets notes.html_content from compiled output. The issue does not mention compiled_pages. If paldocs does not use compiled_pages, this is fine. If it does, the compiler must also update that table. Need to verify whether compiled_pages table exists in Rails schema.
parser.py line count: Issue says "~200 lines" but actual parser.py is 316 lines. The port scope is larger than estimated. compiler.py is 148 lines and sync.py is 112 lines. Total reference code: 576 lines.
BeautifulSoup vs Nokogiri: Parser uses bs4 (BeautifulSoup). Constraint says "Use Nokogiri" which is correct -- Nokogiri is the Ruby equivalent. Nokogiri is available as a transitive dependency via Rails (loofah), confirmed in Gemfile.lock. However, it is NOT in the explicit Gemfile. [BODY] Add constraint: "Add nokogiri to Gemfile explicitly if needed for direct require, or confirm transitive dependency suffices."
Destroy action: The existing Api::NotesController#destroy already does Block.where(note_id: note.id).delete_all -- this bypasses ActiveRecord callbacks (uses delete_all not destroy_all). Not a concern for this ticket but worth noting.

Borderline -- no decomposition needed, but monitor.

File targets: 4 (2 new + 1 modified + references). All in 1 repo. Under threshold.
AC count: 6 + 1 missing = 7. Over the 5 threshold, but all are tightly coupled to a single concern (block parsing on note write).
Estimated agent time: ~10 minutes. The 576 lines of Python reference code must be understood and ported, plus tests. This exceeds the 5-minute rule.
Recommendation: The 8-point estimate is appropriate for the scope. The work is cohesive (single service extraction pattern). Decomposition into sub-tickets would create artificial coupling overhead. However, the agent implementing this should be given clear phasing guidance: (1) port parser + compiler as services with unit tests, (2) wire into controller + integration tests.

[BODY] Fix parser.py line count: "~200 lines" should be "~316 lines" (total reference: 576 lines across parser.py, compiler.py, sync.py).
[BODY] Add from_template to File Targets and add AC: "Api::NotesController#from_template stores Block rows via parser".
[BODY] Reword AC 5 about NOTIFY -- the Rails embedding path is via ActiveRecord after_save callback + Solid Queue, not Postgres NOTIFY. Both coexist on the shared DB but the agent should target the Rails path.
[BODY] Add note about compiled_pages table -- clarify whether the Ruby port needs to update compiled_pages or if that table is pal-e-docs-only.
[SCOPE] Create architecture note arch-rails-api for the Rails API component. Multiple board items reference this label.

Verdict: NEEDS_REFINEMENT