Review: Port block parser from Python to Ruby (re-review)
Verdict: READY
Re-review of board item #1599 after scope refinement. Previous review:
review-1599-2026-06-24 (NEEDS_REFINEMENT, 4 BODY fixes). All 4 BODY fixes have been addressed.Template Completeness
- [x] Type -- Feature
- [x] Lineage -- Spike #75 (PR #76)
- [x] Repo -- ldraney/paldocs
- [x] User Story -- MCP cutover operator story
- [x] Context -- explains gap clearly, now includes correct embedding path and reference code size
- [x] File Targets -- 4 targets listed (including from_template)
- [x] Feature Flag -- none (correct, internal infrastructure)
- [x] Acceptance Criteria -- 7 criteria
- [x] Test Expectations -- 4 expectations with correct RSpec run command
- [x] Constraints -- 4 constraints listed (including explicit embedding path constraint)
- [x] Checklist -- standard
- [x] Related -- 3 references
Traceability
- [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 -- arch-rails-api note still does not exist. Search returns zero results. This is a cross-cutting gap affecting multiple board items (#1568, #1569, #1570, #1599, #1600, #1601, #1602). Not a blocker for this ticket individually -- the label is correct and the work is well-scoped without the note. [SCOPE] Create architecture note arch-rails-api for the Rails API component.
- [x] Forgejo issue -- https://forgejo.tail5b443a.ts.net/ldraney/paldocs/issues/78, state: open
File Targets
- [x]
app/services/block_parser.rb(new) --app/services/directory exists (contains ollama_client.rb). Standard Rails convention. OK. - [x]
app/services/block_compiler.rb(new) -- same directory. OK. - [x]
app/controllers/api/notes_controller.rb-- verified at HEAD. Hascreate(line 85),update(line 167), andfrom_template(line 126). None currently calls any block parsing. Confirmed gap matches issue description. - [x] Reference: pal-e-docs blocks/ Python files -- verified in prior review (parser.py=316, compiler.py=148, sync.py=112). Repo not accessible via current API token but files were confirmed in review-1599-2026-06-24.
Previous BODY Fixes -- Resolution Check
- [x] Fix 1: Add from_template -- RESOLVED. from_template now listed in File Targets, AC 5 added ("Api::NotesController#from_template stores Block rows for the created note"), integration test added ("note from_template via API -> blocks exist").
- [x] Fix 2: Embedding path (NOTIFY vs after_save) -- RESOLVED. AC 6 now correctly reads "Block#after_save :enqueue_embedding callback fires for new/updated blocks (existing Rails mechanism via Solid Queue)". Context section also states this explicitly. Constraint 3 added: "Embedding path is Block#after_save -> Solid Queue EmbedBlockJob, NOT Postgres NOTIFY".
- [x] Fix 3: parser.py line count -- RESOLVED. Context now correctly states "~576 lines across 3 Python files (parser.py=316, compiler.py=148, sync.py=112)".
- [x] Fix 4: Test command -- RESOLVED. Run command is now "bundle exec rspec spec/services/ spec/requests/api/notes_spec.rb".
Repo Placement
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.
Dependencies
- Upstream (done): Issue #57 (API namespace + Notes controller) -- merged. Provides the controller to modify. Controller verified at HEAD with create, update, from_template actions.
- Upstream (done): Issue #52 (Embedding pipeline) -- merged. Block model has
after_save :enqueue_embeddingcallback withneeds_embedding?guard (fires on content/block_type change or new record). EmbedBlockJob enqueues via Solid Queue. - Upstream (done): Issue #49 (Schema ownership) -- blocks table owned by Rails.
- Downstream: Issue #61 (Hybrid search) depends on blocks being created correctly.
- No blockers: In-progress items (#56 Sprint planning UI, #22 Deploy paldocs, #75 MCP compatibility spike) are all independent of this ticket.
Acceptance Criteria
- [x] AC 1: BlockParser.call(html_content) returns array of block hashes -- testable, clear.
- [x] AC 2: BlockCompiler.call(blocks) reconstructs HTML from block array -- testable, clear.
- [x] AC 3: Api::NotesController#create stores Block rows -- testable, clear.
- [x] AC 4: Api::NotesController#update re-parses when html_content changes -- testable, clear. Controller already tracks content_changed boolean.
- [x] AC 5: Api::NotesController#from_template stores Block rows -- testable, clear. NEW in this revision.
- [x] AC 6: Block#after_save :enqueue_embedding callback fires -- testable, correct mechanism. Verified: Block model has this callback with needs_embedding? guard.
- [x] AC 7: Embedding worker picks up blocks -- testable, will work automatically via existing callback.
All 7 AC are testable by an agent. The test expectations cover all paths (unit round-trip, block types, integration for create and from_template).
Blast Radius
- compiled_pages table: Exists in the Rails DB schema (inherited from pal-e-docs migration) but has NO corresponding Rails model. The Python sync.py writes to compiled_pages, but the Rails app does not use it -- the toc and section endpoints work directly from Block rows. NOT a concern for this ticket. The compiled_pages table is a pal-e-docs artifact that can be deprecated in a future cleanup ticket.
- Nokogiri availability: Constraint says "already in Gemfile" -- Nokogiri is NOT in the explicit Gemfile but IS available as a transitive dependency (v1.19.4 via actionview/loofah/rails-html-sanitizer in Gemfile.lock, 21 references). The agent can
require 'nokogiri'without adding it to the Gemfile. Minor wording inaccuracy but does not affect implementation. - destroy action: The existing destroy action uses
Block.where(note_id: note.id).delete_allwhich bypasses ActiveRecord callbacks. This is pre-existing and not affected by this ticket. - BeautifulSoup to Nokogiri: The port from bs4 to Nokogiri is a well-understood translation. Both libraries have equivalent HTML parsing APIs.
Decomposition Assessment
No decomposition needed.
- File targets: 3 modified/created + references. All in 1 repo. Under threshold.
- AC count: 7. Over the 5 threshold, but all are tightly coupled to a single concern (block parsing on note write). Splitting would create artificial coupling overhead.
- Estimated agent time: ~8-10 minutes. Exceeds 5-minute rule, but the 8-point estimate accounts for this. The work is cohesive -- a single service extraction pattern.
- Phasing guidance from prior review still applies: (1) port parser + compiler as services with unit tests, (2) wire into controller + integration tests.
Recommendation
[SCOPE]Create architecture notearch-rails-apifor the Rails API component. Cross-cutting gap affecting 7+ board items. Not a blocker for this specific ticket.
No BODY or LABEL fixes needed. All previous NEEDS_REFINEMENT items have been resolved. Ticket is ready for implementation.