7e-1: Source-of-truth cutover -- note writes parse to blocks first #110

Merged

forgejo_admin merged 1 commit from 109-7e-1-source-of-truth-cutover-note-writes into main

2026-03-08 07:05:21 +00:00

forgejo_admin commented

2026-03-08 06:40:12 +00:00

Contributor

Summary

Make blocks the canonical representation for note content. create_note() and update_note() now parse HTML to blocks via parse_html_to_blocks(), store them, then recompile html_content from blocks. This ensures block reads (get_section, get_toc, list_blocks) are never stale after a note write via the original API.

Changes

src/pal_e_docs/blocks/sync.py (new) -- Shared recompile() and parse_and_store_blocks() helpers extracted from the duplicated _recompile() logic
src/pal_e_docs/blocks/__init__.py -- Export new recompile and parse_and_store_blocks
src/pal_e_docs/routes/blocks.py -- Replace local _recompile() with shared recompile() import; removed hashlib and CompiledPage imports no longer needed
src/pal_e_docs/routes/notes.py -- Wire parse_and_store_blocks() into create_note() and update_note() when html_content is provided; revision stores compiled output
tests/test_note_block_sync.py (new) -- 14 tests covering create, update, metadata-only update, empty content, round-trip consistency, TOC/section integration

Test Plan

14 new tests in test_note_block_sync.py all pass
Full suite: 503 tests pass, 0 failures
Ruff lint + format clean
Run pytest tests/ -v to verify

Review Checklist

Code follows existing patterns (reuses _recompile logic, extracted to shared module)
No unrelated changes
Tests cover all acceptance criteria from issue #109
Metadata-only updates do NOT trigger block processing
Round-trip consistency verified (create -> block read -> html update -> block read)

Plan: plan-2026-02-26-tf-modularize-postgres
Forgejo issue: #109

## Summary Make blocks the canonical representation for note content. `create_note()` and `update_note()` now parse HTML to blocks via `parse_html_to_blocks()`, store them, then recompile `html_content` from blocks. This ensures block reads (get_section, get_toc, list_blocks) are never stale after a note write via the original API. ## Changes - **`src/pal_e_docs/blocks/sync.py`** (new) -- Shared `recompile()` and `parse_and_store_blocks()` helpers extracted from the duplicated `_recompile()` logic - **`src/pal_e_docs/blocks/__init__.py`** -- Export new `recompile` and `parse_and_store_blocks` - **`src/pal_e_docs/routes/blocks.py`** -- Replace local `_recompile()` with shared `recompile()` import; removed `hashlib` and `CompiledPage` imports no longer needed - **`src/pal_e_docs/routes/notes.py`** -- Wire `parse_and_store_blocks()` into `create_note()` and `update_note()` when `html_content` is provided; revision stores compiled output - **`tests/test_note_block_sync.py`** (new) -- 14 tests covering create, update, metadata-only update, empty content, round-trip consistency, TOC/section integration ## Test Plan - [x] 14 new tests in `test_note_block_sync.py` all pass - [x] Full suite: 503 tests pass, 0 failures - [x] Ruff lint + format clean - Run `pytest tests/ -v` to verify ## Review Checklist - [x] Code follows existing patterns (reuses `_recompile` logic, extracted to shared module) - [x] No unrelated changes - [x] Tests cover all acceptance criteria from issue #109 - [x] Metadata-only updates do NOT trigger block processing - [x] Round-trip consistency verified (create -> block read -> html update -> block read) ## Related - Plan: `plan-2026-02-26-tf-modularize-postgres` - Forgejo issue: #109

forgejo_admin added 1 commit

2026-03-08 06:40:12 +00:00

7e-1: Source-of-truth cutover -- note writes parse to blocks first

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

56c8af0625

Make blocks the canonical representation for note content. create_note()
and update_note() now parse HTML to blocks, store them, then recompile
html_content from blocks. This ensures block reads are never stale after
a note write.

Closes #109

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-03-08 06:50:56 +00:00

Author

Contributor

PR #110 Review

BLOCKERS

None.

NITS

create_note sets html_content twice on the Note object. In notes.py line 273, the Note is constructed with html_content=body.html_content (the raw input). Then at line 287, parse_and_store_blocks() overwrites note.html_content with the compiled output. This works correctly -- the compiled version wins before commit -- but the initial assignment is technically wasted work. A minor clarity improvement would be to set html_content="" (or None) at construction and let parse_and_store_blocks be the sole writer. Not blocking since the end result is correct.
Test helper sessions are opened outside the request lifecycle. The _count_blocks(), _get_blocks(), and _get_compiled_page() helpers in test_note_block_sync.py open their own TestingSessionLocal() sessions. This works because the test DB is shared, but it relies on the commit having completed before the helper runs. This is fine for these tests (the client calls do commit), but worth noting for future test authors. Not blocking.
_get_compiled_page returns a detached ORM object. The helper closes the session after returning the CompiledPage instance. Accessing lazy-loaded relationships on the returned object would fail. This is fine for current usage (only content_hash, toc_json, html are accessed, all column attributes), but could surprise future test authors. Not blocking.

SOP COMPLIANCE

Branch named after issue number (109-7e-1-source-of-truth-cutover-note-writes references issue #109)
PR body has: ## Summary, ## Changes, ## Test Plan, ## Related
Related section references the plan slug (plan-2026-02-26-tf-modularize-postgres)
Tests exist (14 new tests in test_note_block_sync.py, reported 503 total passing)
No secrets, .env files, or credentials committed
No unnecessary file changes -- all 5 files are tightly scoped to the cutover
Commit messages are descriptive (PR title is clear)

Code Quality Notes

Extract-and-share pattern is correct. The _recompile() duplication in routes/blocks.py is cleanly extracted to blocks/sync.py as recompile(). The new parse_and_store_blocks() composes parse, delete, insert, and recompile in the right order.
Revision stores compiled output. Both create_note and update_note now store note.html_content (compiled) in revisions instead of body.html_content (raw). This is the correct behavior for source-of-truth cutover -- revisions should reflect what blocks actually produce.
Empty content guard is correct. if body.html_content: (falsy check) in create_note skips block processing for empty strings, which is the right behavior. In update_note, if body.html_content is not None: correctly triggers block processing even for empty string (to clear blocks), which is also correct and tested.
Flush ordering is sound. The note is flushed (gets an id) before parse_and_store_blocks runs, which needs note.id for block insertion. The recompile at the end flushes again. Commit happens in the route after all operations complete.

VERDICT: APPROVED

## PR #110 Review ### BLOCKERS None. ### NITS 1. **`create_note` sets `html_content` twice on the Note object.** In `notes.py` line 273, the Note is constructed with `html_content=body.html_content` (the raw input). Then at line 287, `parse_and_store_blocks()` overwrites `note.html_content` with the compiled output. This works correctly -- the compiled version wins before commit -- but the initial assignment is technically wasted work. A minor clarity improvement would be to set `html_content=""` (or `None`) at construction and let `parse_and_store_blocks` be the sole writer. Not blocking since the end result is correct. 2. **Test helper sessions are opened outside the request lifecycle.** The `_count_blocks()`, `_get_blocks()`, and `_get_compiled_page()` helpers in `test_note_block_sync.py` open their own `TestingSessionLocal()` sessions. This works because the test DB is shared, but it relies on the commit having completed before the helper runs. This is fine for these tests (the client calls do commit), but worth noting for future test authors. Not blocking. 3. **`_get_compiled_page` returns a detached ORM object.** The helper closes the session after returning the `CompiledPage` instance. Accessing lazy-loaded relationships on the returned object would fail. This is fine for current usage (only `content_hash`, `toc_json`, `html` are accessed, all column attributes), but could surprise future test authors. Not blocking. ### SOP COMPLIANCE - [x] Branch named after issue number (`109-7e-1-source-of-truth-cutover-note-writes` references issue #109) - [x] PR body has: ## Summary, ## Changes, ## Test Plan, ## Related - [x] Related section references the plan slug (`plan-2026-02-26-tf-modularize-postgres`) - [x] Tests exist (14 new tests in `test_note_block_sync.py`, reported 503 total passing) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes -- all 5 files are tightly scoped to the cutover - [x] Commit messages are descriptive (PR title is clear) ### Code Quality Notes - **Extract-and-share pattern is correct.** The `_recompile()` duplication in `routes/blocks.py` is cleanly extracted to `blocks/sync.py` as `recompile()`. The new `parse_and_store_blocks()` composes parse, delete, insert, and recompile in the right order. - **Revision stores compiled output.** Both `create_note` and `update_note` now store `note.html_content` (compiled) in revisions instead of `body.html_content` (raw). This is the correct behavior for source-of-truth cutover -- revisions should reflect what blocks actually produce. - **Empty content guard is correct.** `if body.html_content:` (falsy check) in `create_note` skips block processing for empty strings, which is the right behavior. In `update_note`, `if body.html_content is not None:` correctly triggers block processing even for empty string (to clear blocks), which is also correct and tested. - **Flush ordering is sound.** The note is flushed (gets an `id`) before `parse_and_store_blocks` runs, which needs `note.id` for block insertion. The `recompile` at the end flushes again. Commit happens in the route after all operations complete. ### VERDICT: APPROVED