Created
March 2, 2026 20:00
-
-
Save kkurian/647df8ca94643346998c00ec6944e69c to your computer and use it in GitHub Desktop.
Notion connector code review
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
| <!DOCTYPE html> | |
| <html lang="en"> | |
| <head> | |
| <meta charset="UTF-8"> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | |
| <title>Connector Review: Notion</title> | |
| <style> | |
| :root { | |
| --bg: #0d1117; --fg: #c9d1d9; --accent: #58a6ff; | |
| --red: #f85149; --orange: #d29922; --green: #3fb950; --purple: #bc8cff; | |
| --card-bg: #161b22; --border: #30363d; --muted: #8b949e; | |
| } | |
| * { box-sizing: border-box; margin: 0; padding: 0; } | |
| body { background: var(--bg); color: var(--fg); font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Helvetica, Arial, sans-serif; line-height: 1.6; padding: 2rem; max-width: 1200px; margin: 0 auto; } | |
| h1 { color: var(--accent); font-size: 1.8rem; border-bottom: 1px solid var(--border); padding-bottom: 0.5rem; margin-bottom: 1.5rem; } | |
| h2 { color: var(--purple); font-size: 1.4rem; margin: 2rem 0 1rem; border-bottom: 1px solid var(--border); padding-bottom: 0.3rem; } | |
| h3 { color: var(--fg); font-size: 1.15rem; margin: 1.5rem 0 0.5rem; } | |
| h4 { color: var(--muted); font-size: 1rem; margin: 1rem 0 0.3rem; } | |
| p, li { color: var(--fg); margin-bottom: 0.5rem; } | |
| a { color: var(--accent); text-decoration: none; } | |
| a:hover { text-decoration: underline; } | |
| code { background: #1c2129; padding: 0.15em 0.4em; border-radius: 4px; font-size: 0.9em; color: var(--orange); } | |
| pre { background: #1c2129; padding: 1rem; border-radius: 8px; overflow-x: auto; margin: 0.5rem 0 1rem; border: 1px solid var(--border); } | |
| pre code { background: none; padding: 0; color: var(--fg); } | |
| table { width: 100%; border-collapse: collapse; margin: 0.5rem 0 1rem; } | |
| th { background: #1c2129; color: var(--accent); text-align: left; padding: 0.5rem 0.75rem; border: 1px solid var(--border); font-weight: 600; } | |
| td { padding: 0.5rem 0.75rem; border: 1px solid var(--border); vertical-align: top; } | |
| .card { background: var(--card-bg); border: 1px solid var(--border); border-radius: 8px; padding: 1.25rem; margin-bottom: 1rem; } | |
| .badge { display: inline-block; padding: 0.15em 0.6em; border-radius: 12px; font-size: 0.8rem; font-weight: 600; } | |
| .badge-critical { background: var(--red); color: #fff; } | |
| .badge-high { background: var(--orange); color: #000; } | |
| .badge-medium { background: var(--purple); color: #fff; } | |
| .badge-low { background: var(--green); color: #000; } | |
| .badge-pass { background: var(--green); color: #000; } | |
| .badge-fail { background: var(--red); color: #fff; } | |
| .badge-partial { background: var(--orange); color: #000; } | |
| .summary-grid { display: grid; grid-template-columns: repeat(auto-fit, minmax(200px, 1fr)); gap: 1rem; margin: 1rem 0; } | |
| .summary-box { background: var(--card-bg); border: 1px solid var(--border); border-radius: 8px; padding: 1rem; text-align: center; } | |
| .summary-box .num { font-size: 2rem; font-weight: 700; } | |
| .summary-box .label { color: var(--muted); font-size: 0.85rem; } | |
| .fact-table { font-size: 0.9rem; } | |
| .fact-table td:first-child { color: var(--accent); white-space: nowrap; } | |
| .fact-table td:nth-child(2) { color: var(--orange); font-weight: 600; white-space: nowrap; } | |
| .fact-table td:nth-child(3) { color: var(--fg); } | |
| .toc { background: var(--card-bg); border: 1px solid var(--border); border-radius: 8px; padding: 1rem 1.5rem; margin-bottom: 2rem; } | |
| .toc ul { list-style: none; padding-left: 1rem; } | |
| .toc > ul { padding-left: 0; } | |
| .toc li { margin: 0.3rem 0; } | |
| .note { background: #1c2129; border-left: 3px solid var(--accent); padding: 0.75rem 1rem; margin: 0.5rem 0 1rem; border-radius: 0 4px 4px 0; font-size: 0.9rem; } | |
| details { margin: 0.5rem 0; } | |
| summary { cursor: pointer; color: var(--accent); font-weight: 600; } | |
| summary:hover { text-decoration: underline; } | |
| </style> | |
| </head> | |
| <body> | |
| <h1>Connector Review: Notion</h1> | |
| <p style="color: var(--muted);">Date: 2026-03-02 · Branch: kk/notion-omnibus-review · References: Snowflake, Looker, Chorus</p> | |
| <nav class="toc"> | |
| <h3>Contents</h3> | |
| <ul> | |
| <li><a href="#summary">Issue Summary</a></li> | |
| <li><a href="#issues">Consolidated Issue List</a></li> | |
| <li><a href="#arch-patterns">Architectural Pattern Analysis</a></li> | |
| <li><a href="#conformance">Structural & Behavioral Conformance</a></li> | |
| <li><a href="#code-smells">Semantic Clarity — Code Smells</a></li> | |
| <li><a href="#assumptions">Assumption Validation</a></li> | |
| <li><a href="#test-coverage">Test Coverage Analysis</a></li> | |
| <li><a href="#dependencies">Dependencies</a></li> | |
| </ul> | |
| </nav> | |
| <!-- ============================================================ --> | |
| <!-- SUMMARY GRID --> | |
| <!-- ============================================================ --> | |
| <h2 id="summary">Issue Summary</h2> | |
| <div class="summary-grid"> | |
| <div class="summary-box"> | |
| <div class="num" style="color: var(--red);">1</div> | |
| <div class="label">Critical</div> | |
| </div> | |
| <div class="summary-box"> | |
| <div class="num" style="color: var(--orange);">7</div> | |
| <div class="label">High</div> | |
| </div> | |
| <div class="summary-box"> | |
| <div class="num" style="color: var(--purple);">8</div> | |
| <div class="label">Medium</div> | |
| </div> | |
| <div class="summary-box"> | |
| <div class="num" style="color: var(--green);">8</div> | |
| <div class="label">Low</div> | |
| </div> | |
| </div> | |
| <div class="note"> | |
| <strong>Overall assessment:</strong> The Notion connector is structurally complete and follows established patterns well — 14/15 structural checks pass and 10/10 behavioral checks pass. The critical issue is a block-type discovery gap that silently misses child pages nested in certain containers. The high-severity findings center on duplicated sync logic and missing test coverage for production paths (Temporal workflows, routes, shared sync loop). | |
| </div> | |
| <!-- ============================================================ --> | |
| <!-- CONSOLIDATED ISSUE LIST --> | |
| <!-- ============================================================ --> | |
| <h2 id="issues">Consolidated Issue List</h2> | |
| <h3>Critical</h3> | |
| <table> | |
| <tr><th style="width:3rem">ID</th><th>Description</th><th style="width:5rem">Source</th><th>File(s)</th><th style="width:4rem">Detail</th></tr> | |
| <tr><td>C1</td><td><code>_PAGE_CONTAINER_TYPES</code> is incomplete — child pages nested inside callouts, quotes, list items, and toggleable headings are silently missed during tree crawl</td><td>Agent E</td><td><code>client.py:34</code></td><td><a href="#detail-C1">details</a></td></tr> | |
| </table> | |
| <h3>High</h3> | |
| <table> | |
| <tr><th style="width:3rem">ID</th><th>Description</th><th style="width:5rem">Source</th><th>File(s)</th><th style="width:4rem">Detail</th></tr> | |
| <tr><td>H1</td><td>Core sync iteration logic duplicated between <code>notion_sync_service.py</code> and <code>sync.py</code> — bugs fixed in one won't propagate to the other</td><td>Agent D/E</td><td><code>notion_sync_service.py:110-226</code>, <code>sync.py:55-207</code></td><td><a href="#detail-H1">details</a></td></tr> | |
| <tr><td>H2</td><td>Missing <code>datetime.fromisoformat</code> error handling in Temporal sync path — invalid timestamp from Notion API crashes the entire org sync</td><td>Agent E</td><td><code>notion_sync_service.py:158</code></td><td><a href="#detail-H2">details</a></td></tr> | |
| <tr><td>H3</td><td>Incremental sync skips rely on <code>last_edited_time</code> which Notion rounds to the nearest minute — rapid edits after sync can be missed</td><td>Agent E</td><td><code>notion_sync_service.py:156-161</code>, <code>sync.py:139</code></td><td><a href="#detail-H3">details</a></td></tr> | |
| <tr><td>H4</td><td><code>NotionSyncTrigger</code> wired via DI but never called from routes — new page trees don't sync until the next hourly cron</td><td>Agent D</td><td><code>routes.py:52-54</code>, <code>app.py:52</code></td><td><a href="#detail-H4">details</a></td></tr> | |
| <tr><td>H5</td><td>Shared sync loop (<code>sync.py</code>, 207 lines) has zero direct test coverage</td><td>Agent F</td><td><code>sync.py</code></td><td><a href="#detail-H5">details</a></td></tr> | |
| <tr><td>H6</td><td>Temporal workflows and activities (<code>RunNotionSync*</code>, <code>TriggerNotionSync*</code>) have zero test coverage</td><td>Agent F</td><td><code>run_notion_sync_workflow.py</code>, <code>trigger_notion_sync_workflow.py</code></td><td><a href="#detail-H6">details</a></td></tr> | |
| <tr><td>H7</td><td>Routes module (8 HTTP endpoints) entirely untested — request parsing, error mapping, and DI wiring unverified</td><td>Agent F</td><td><code>routes.py</code></td><td><a href="#detail-H7">details</a></td></tr> | |
| </table> | |
| <h3>Medium</h3> | |
| <table> | |
| <tr><th style="width:3rem">ID</th><th>Description</th><th style="width:5rem">Source</th><th>File(s)</th><th style="width:4rem">Detail</th></tr> | |
| <tr><td>M1</td><td>Magic timeout constants (30s, 60s, 120s) scattered across 5+ files with no named constants</td><td>Agent E</td><td><code>client.py:94</code>, <code>connection.py:85</code>, <code>cli.py:120</code>, etc.</td><td><a href="#detail-M1">details</a></td></tr> | |
| <tr><td>M2</td><td><code>extract_page_title()</code> returns <code>""</code> instead of <code>None</code> for missing titles — sentinel value masks absence</td><td>Agent E</td><td><code>transform.py:73</code></td><td><a href="#detail-M2">details</a></td></tr> | |
| <tr><td>M3</td><td>Secret Manager secrets orphaned on disconnect with no cleanup mechanism</td><td>Agent E</td><td><code>connection.py:268-270</code></td><td><a href="#detail-M3">details</a></td></tr> | |
| <tr><td>M4</td><td>Per-sync client creation means independent rate limiters — concurrent syncs for same org would exceed Notion's 3 req/s</td><td>Agent E</td><td><code>client.py:92-94</code></td><td><a href="#detail-M4">details</a></td></tr> | |
| <tr><td>M5</td><td><code>update_page()</code> endpoint completely untested despite being a user-facing PATCH</td><td>Agent F</td><td><code>resource_service.py:186-217</code></td><td><a href="#detail-M5">details</a></td></tr> | |
| <tr><td>M6</td><td>App factory wiring (<code>create_notion_app()</code>) untested — no verification of dependency overrides</td><td>Agent F</td><td><code>app.py:20-62</code></td><td><a href="#detail-M6">details</a></td></tr> | |
| <tr><td>M7</td><td>Connection service error/rollback paths untested (Secret Manager failure, DB commit failure)</td><td>Agent F</td><td><code>connection.py:101-110, 143-151, 203-210</code></td><td><a href="#detail-M7">details</a></td></tr> | |
| <tr><td>M8</td><td>CLI <code>sync</code> command (largest function in cli.py, 100+ lines) entirely untested</td><td>Agent F</td><td><code>cli.py:91-194</code></td><td><a href="#detail-M8">details</a></td></tr> | |
| </table> | |
| <h3>Low</h3> | |
| <table> | |
| <tr><th style="width:3rem">ID</th><th>Description</th><th style="width:5rem">Source</th><th>File(s)</th><th style="width:4rem">Detail</th></tr> | |
| <tr><td>L1</td><td><code>PageSyncEvent.action</code> uses magic strings (<code>"synced"</code>, <code>"skipped_excluded"</code>) instead of a <code>StrEnum</code></td><td>Agent E</td><td><code>sync.py:34</code></td><td><a href="#detail-L1">details</a></td></tr> | |
| <tr><td>L2</td><td>Hardcoded <code>"success"</code> string instead of <code>IngestionStatus.SUCCESS.value</code></td><td>Agent E</td><td><code>store.py:215</code></td><td><a href="#detail-L2">details</a></td></tr> | |
| <tr><td>L3</td><td><code>get_sync_trigger</code> placeholder has <code>-> None</code> return type annotation (should be <code>-> NotionSyncTrigger</code>)</td><td>Agent E</td><td><code>routes.py:52-54</code></td><td><a href="#detail-L3">details</a></td></tr> | |
| <tr><td>L4</td><td>Prometheus metrics port <code>8414</code> hardcoded as magic number instead of in settings</td><td>Agent E</td><td><code>notion_sync_worker_main.py:134</code></td><td><a href="#detail-L4">details</a></td></tr> | |
| <tr><td>L5</td><td>Notion base URL hardcoded in <code>store.py</code> and <code>models.py</code> despite <code>settings.app_base_url</code> existing</td><td>Agent E</td><td><code>store.py:60</code>, <code>temporal/models.py:41</code></td><td><a href="#detail-L5">details</a></td></tr> | |
| <tr><td>L6</td><td>f-string interpolation in <code>logger.info()</code> instead of lazy <code>%</code>-style formatting</td><td>Agent E</td><td><code>notion_sync_worker_main.py:136</code></td><td><a href="#detail-L6">details</a></td></tr> | |
| <tr><td>L7</td><td>Custom UUID namespace diverges from codebase's <code>NAMESPACE_URL</code>/<code>NAMESPACE_DNS</code> pattern without documentation</td><td>Agent E</td><td><code>temporal/models.py:9</code></td><td><a href="#detail-L7">details</a></td></tr> | |
| <tr><td>L8</td><td>No 1Password <code>.env.op</code> entries for Notion (acceptable for API-key integration, noted for completeness)</td><td>Agent D</td><td><code>src/app-frontend/.env.op</code></td><td><a href="#detail-L8">details</a></td></tr> | |
| </table> | |
| <!-- ============================================================ --> | |
| <!-- ARCHITECTURAL PATTERN ANALYSIS --> | |
| <!-- ============================================================ --> | |
| <h2 id="arch-patterns">Architectural Pattern Analysis</h2> | |
| <div class="card" id="detail-C1"> | |
| <h3>Pattern: Block Tree Discovery</h3> | |
| <h4>Reference Facts</h4> | |
| <p>Not applicable — Snowflake/Looker/Chorus don't crawl nested content trees. This is a Notion-specific pattern.</p> | |
| <h4>Implementation Facts</h4> | |
| <table class="fact-table"> | |
| <tr><td>NotionClient.crawl_page_tree()</td><td>discovers</td><td>child pages via BFS, level-by-level (<code>client.py:152-226</code>)</td></tr> | |
| <tr><td>NotionClient._find_child_pages_in_blocks()</td><td>filters</td><td>blocks by <code>_PAGE_CONTAINER_TYPES</code> set to decide which to recurse into (<code>client.py:231-256</code>)</td></tr> | |
| <tr><td>_PAGE_CONTAINER_TYPES</td><td>contains</td><td>only <code>toggle</code>, <code>column</code>, <code>column_list</code>, <code>synced_block</code> (<code>client.py:34</code>)</td></tr> | |
| </table> | |
| <h4>Assessment</h4> | |
| <p><span class="badge badge-critical">Critical</span> The Notion API documents that <code>bulleted_list_item</code>, <code>callout</code>, <code>numbered_list_item</code>, <code>paragraph</code>, <code>quote</code>, <code>template</code>, <code>to_do</code>, and toggleable headings (<code>heading_1/2/3</code> with <code>is_toggleable=true</code>) all support children. A user can nest a <code>child_page</code> inside any of these. The current filter silently skips them, causing those pages and all their descendants to be missing from syncs.</p> | |
| <p><strong>Fix:</strong> Either expand <code>_PAGE_CONTAINER_TYPES</code> to include all container types, or simplify the logic: if <code>block.get("has_children")</code> is <code>True</code> and the block type is not <code>child_page</code>/<code>child_database</code>, recurse into it. The latter approach is more future-proof against new Notion block types.</p> | |
| </div> | |
| <div class="card" id="detail-H1"> | |
| <h3>Pattern: Shared Sync Loop vs. Inline Duplication</h3> | |
| <h4>Reference Facts</h4> | |
| <table class="fact-table"> | |
| <tr><td>SnowflakeSyncService.run_snowflake_sync()</td><td>contains</td><td>single copy of sync logic (<code>snowflake_sync_service.py:168-251</code>)</td></tr> | |
| <tr><td>ChorusSyncer.sync()</td><td>contains</td><td>single copy of sync logic, shared by all callers (<code>sync.py:84-398</code>)</td></tr> | |
| </table> | |
| <h4>Implementation Facts</h4> | |
| <table class="fact-table"> | |
| <tr><td>sync_page_tree()</td><td>implements</td><td>crawl-filter-extract-ingest loop with callback (<code>sync.py:55-207</code>)</td></tr> | |
| <tr><td>sync_page_tree()</td><td>wraps</td><td>datetime.fromisoformat in try/except (<code>sync.py:132-135</code>)</td></tr> | |
| <tr><td>NotionSyncService.run_notion_sync()</td><td>reimplements</td><td>same crawl-filter-extract-ingest loop inline (<code>notion_sync_service.py:110-226</code>)</td></tr> | |
| <tr><td>NotionSyncService.run_notion_sync()</td><td>omits</td><td>datetime.fromisoformat error handling (<code>notion_sync_service.py:158</code>)</td></tr> | |
| </table> | |
| <h4>Verb Comparison</h4> | |
| <p>Both copies use the same verbs: crawls, filters (exclusions), compares (timestamps), extracts (blocks), transforms (to text), ingests (to Briefs), upserts (content records). The duplication is nearly 1:1, with the critical difference that <code>sync.py</code> handles timestamp parsing errors defensively while <code>notion_sync_service.py</code> does not.</p> | |
| <h4>Assessment</h4> | |
| <p><span class="badge badge-high">High</span> The <code>sync.py</code> comment says "Used by the CLI (and planned for NotionSyncService migration)." Complete that migration: refactor <code>run_notion_sync</code> to call <code>sync_page_tree()</code> with an appropriate <code>ingest</code> callback. This resolves H1, H2, and H5 simultaneously.</p> | |
| </div> | |
| <div class="card" id="detail-H4"> | |
| <h3>Pattern: API-Triggered Sync on Resource Creation</h3> | |
| <h4>Reference Facts</h4> | |
| <table class="fact-table"> | |
| <tr><td>Snowflake routes.py</td><td>calls</td><td>sync_trigger.sync_resource() after POST /objects/{org_id} (<code>routes.py:153-158</code>)</td></tr> | |
| <tr><td>SnowflakeSyncTrigger</td><td>starts</td><td>RunSnowflakeSyncWorkflow immediately with USE_EXISTING policy (<code>sync_trigger.py:57-68</code>)</td></tr> | |
| </table> | |
| <h4>Implementation Facts</h4> | |
| <table class="fact-table"> | |
| <tr><td>create_notion_app()</td><td>wires</td><td>NotionSyncTrigger via dependency_overrides (<code>app.py:52,58</code>)</td></tr> | |
| <tr><td>routes.get_sync_trigger()</td><td>defines</td><td>placeholder returning None (<code>routes.py:52-54</code>)</td></tr> | |
| <tr><td>routes.add_page()</td><td>does NOT call</td><td>sync_trigger after resource creation (<code>routes.py:127-138</code>)</td></tr> | |
| </table> | |
| <h4>Assessment</h4> | |
| <p><span class="badge badge-high">High</span> The sync trigger infrastructure is fully wired but the route never calls it. New page trees sit idle until the next hourly cron. The Snowflake connector triggers an immediate sync after resource creation — Notion should do the same. Fix: add <code>sync_trigger = Depends(get_sync_trigger)</code> to the <code>add_page</code> handler and call <code>await sync_trigger.sync_resource(...)</code> after the resource is created.</p> | |
| </div> | |
| <h3>Data Transformation Analysis</h3> | |
| <div class="card"> | |
| <h4>Transformer Verb Comparison</h4> | |
| <table> | |
| <tr><th>Component</th><th>Verbs</th><th>Assessment</th></tr> | |
| <tr><td>Chorus CallTransformer</td><td>parses JSON, maps speaker emails, extracts paragraphs, parses datetimes</td><td>Substantive shape change</td></tr> | |
| <tr><td>Snowflake/Looker DocumentTransformer</td><td>encodes str→bytes, resolves title/filename/content_type defaults</td><td>Substantive type conversion</td></tr> | |
| <tr><td>Notion <code>blocks_to_text()</code> + DocumentTransformer</td><td>extracts rich_text, formats by block type, recurses into children, joins lines, then encodes→bytes</td><td><span class="badge badge-pass">Substantive</span> Genuine shape change from nested block tree to flat markdown</td></tr> | |
| </table> | |
| <p>The Notion connector's transformation pipeline does real work. The <code>blocks_to_text()</code> function at <code>transform.py:29-65</code> converts a deeply nested Notion block tree (with typed blocks, rich_text arrays, and child hierarchies) into flat markdown with appropriate formatting. This is comparable in complexity to Chorus's call transcript extraction.</p> | |
| </div> | |
| <h3>Resource Lifecycle Comparison</h3> | |
| <div class="card"> | |
| <table> | |
| <tr><th>Aspect</th><th>Reference (Snowflake)</th><th>Notion</th><th>Match?</th></tr> | |
| <tr><td>API-route HTTP client</td><td>Shared <code>http_client</code> injected at app creation</td><td>Same: shared <code>http_client</code> from main app (<code>app.py:61</code>)</td><td><span class="badge badge-pass">Yes</span></td></tr> | |
| <tr><td>Sync-time client</td><td>Dedicated connector client per sync, cleaned up via context manager</td><td>Dedicated <code>NotionClient</code> per sync with rate-limited transport, <code>async with</code> cleanup (<code>notion_sync_service.py:126</code>)</td><td><span class="badge badge-pass">Yes</span></td></tr> | |
| <tr><td>Worker HTTP client</td><td>Shared <code>httpx.AsyncClient</code>, closed on shutdown</td><td>Same: <code>httpx.AsyncClient(timeout=60.0)</code> at init, closed in <code>close()</code> (<code>notion_sync_worker_main.py:41,55</code>)</td><td><span class="badge badge-pass">Yes</span></td></tr> | |
| <tr><td>DB sessions</td><td>Short-lived per-operation via <code>async_sessionmaker</code></td><td>Same: <code>async with self.async_session_factory()</code> per DB operation (<code>notion_sync_service.py:79,120,135</code>)</td><td><span class="badge badge-pass">Yes</span></td></tr> | |
| <tr><td>Secret Manager</td><td>Initialized at worker startup, closed on shutdown</td><td>Same pattern (<code>notion_sync_worker_main.py:40,56</code>)</td><td><span class="badge badge-pass">Yes</span></td></tr> | |
| </table> | |
| <p>Resource lifecycle matches the reference pattern exactly. No leaks or lifecycle mismatches detected.</p> | |
| </div> | |
| <h3>End-to-End Feature Completeness</h3> | |
| <div class="card"> | |
| <p><strong>Full data path (discovery → ingestion):</strong> TriggerNotionSyncWorkflow → list_syncs activity → NotionSyncService.list_notion_syncs() → RunNotionSyncWorkflow per resource → NotionClient.create() (credential fetch) → crawl_page_tree() (BFS discovery) → incremental filter (last_edited_time) → get_all_blocks() → blocks_to_text() → ContentContextHandler.handle_content(DocumentContent) → DocumentTransformer → BriefsDocumentIngester → store.upsert_content() → update last_sync_at</p> | |
| <p><strong>All components are wired and do real work.</strong> No dead-end paths or no-op components detected.</p> | |
| <p><strong>Gap:</strong> API-triggered sync after page creation is not implemented (see <a href="#detail-H4">H4</a>).</p> | |
| <p><strong>Not applicable:</strong> OAuth provider registration — Notion uses API key auth, so no <code>register_oauth2_provider()</code> is needed.</p> | |
| </div> | |
| <!-- ============================================================ --> | |
| <!-- STRUCTURAL & BEHAVIORAL CONFORMANCE --> | |
| <!-- ============================================================ --> | |
| <h2 id="conformance">Structural & Behavioral Conformance</h2> | |
| <h3>Structural Compliance</h3> | |
| <table> | |
| <tr><th>Pattern</th><th>Status</th><th>Notes</th></tr> | |
| <tr><td>Models in schemas.py</td><td><span class="badge badge-pass">PASS</span></td><td>All Pydantic request/response models in <code>schemas.py</code>. Temporal models in <code>temporal/models.py</code> (correct Temporal-safe split).</td></tr> | |
| <tr><td>Factory method for client</td><td><span class="badge badge-pass">PASS</span></td><td><code>NotionClient.create()</code> classmethod at <code>client.py:66-101</code>.</td></tr> | |
| <tr><td>Dependency injection</td><td><span class="badge badge-pass">PASS</span></td><td><code>app.py:55-58</code> uses <code>dependency_overrides</code> on placeholder functions. 4 deps wired.</td></tr> | |
| <tr><td>Integration registration</td><td><span class="badge badge-pass">PASS</span></td><td><code>IntegrationTypes.NOTION = "notion"</code> at <code>models.py:80</code>. <code>INTEGRATION_AUTH_CONFIG</code> entry at <code>models.py:157-163</code>.</td></tr> | |
| <tr><td>Store + resource_service</td><td><span class="badge badge-pass">PASS</span></td><td><code>NotionStore</code> with full CRUD. <code>NotionResourceService</code> with add/list/update/delete/exclusions/preview.</td></tr> | |
| <tr><td>Prometheus metrics</td><td><span class="badge badge-pass">PASS</span></td><td><code>notion_resource_operations_total</code> Counter + <code>notion_resource_duration_seconds</code> Histogram in <code>metrics.py</code>.</td></tr> | |
| <tr><td>Frontend tRPC router</td><td><span class="badge badge-pass">PASS</span></td><td>7 procedures in <code>api/router.ts</code>. Zod schemas in <code>schemas.ts</code>. 5 components.</td></tr> | |
| <tr><td>K8s deployment</td><td><span class="badge badge-pass">PASS</span></td><td>Production + PR-preview worker YAMLs present. KEDA ScaledObject for preview scale-to-zero.</td></tr> | |
| <tr><td>Nested BaseSettings with env_prefix</td><td><span class="badge badge-pass">PASS</span></td><td><code>NotionSettings(BaseSettings)</code> with <code>env_prefix="NOTION_"</code>. <code>NotionTemporalSettings</code> with <code>env_prefix="FIDO_"</code> + <code>env_nested_delimiter="__"</code>.</td></tr> | |
| <tr><td>Registered in main Settings</td><td><span class="badge badge-pass">PASS</span></td><td><code>notion: NotionSettings = NotionSettings()</code> nested in main <code>Settings</code> at <code>settings.py:60</code>.</td></tr> | |
| <tr><td>Temporal worker nests connector settings</td><td><span class="badge badge-pass">PASS</span></td><td><code>NotionTemporalSettings</code> nests <code>temporal: BaseTemporalSettings</code> at <code>temporal/settings.py:21</code>.</td></tr> | |
| <tr><td>K8s env vars use double underscore</td><td><span class="badge badge-pass">PASS</span></td><td><code>FIDO_TEMPORAL__TEMPORAL_NAMESPACE</code>, <code>FIDO_TEMPORAL__TEMPORAL_ADDRESS</code>, etc.</td></tr> | |
| <tr><td>No hardcoded production values</td><td><span class="badge badge-pass">PASS</span></td><td>Only default is <code>app_base_url = "https://www.notion.so"</code> (public URL, not a secret).</td></tr> | |
| <tr><td>K8s Secret with secretKeyRef</td><td><span class="badge badge-pass">PASS</span></td><td>References <code>fido-temporal</code> and <code>postgres-secret</code>/<code>preview-postgres-secret</code>.</td></tr> | |
| <tr><td>1Password .env.op entries</td><td><span class="badge badge-fail">N/A</span></td><td>No entries. Acceptable — Notion uses API-key auth stored at connect-time, not OAuth client credentials.</td></tr> | |
| </table> | |
| <h3>Behavioral Compliance</h3> | |
| <table> | |
| <tr><th>Pattern</th><th>Status</th><th>Universal?</th><th>Notes</th></tr> | |
| <tr><td>Credential chain</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>Secret Manager → IntegrationService.get_valid_token() → NotionClient.create(). SELECT FOR UPDATE at <code>connection.py:115-123</code>.</td></tr> | |
| <tr><td>ContentContextHandler</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>DocumentTransformer + BriefsDocumentIngester wired at <code>notion_sync_service.py:67-73</code>.</td></tr> | |
| <tr><td>Two-tier workflow (trigger → run)</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>TriggerNotionSyncWorkflow → RunNotionSyncWorkflow. Matches Snowflake exactly.</td></tr> | |
| <tr><td>Deterministic workflow IDs</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td><code>uuid5</code> with fixed namespace from org_id + page_id at <code>models.py:9-26</code>.</td></tr> | |
| <tr><td>Heartbeat pattern (~10s)</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td><code>asyncio.sleep(10)</code> + <code>activity.heartbeat()</code> at <code>run_notion_sync_workflow.py:37-40</code>.</td></tr> | |
| <tr><td>Sync service protocol</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td><code>NotionSyncServiceProto(Protocol)</code> at <code>models.py:71-76</code>. Decouples workflow from implementation.</td></tr> | |
| <tr><td>Error contract</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td><code>NotionSyncResult(ok, error)</code> + <code>RuntimeError</code> in activity at <code>run_notion_sync_workflow.py:59</code>.</td></tr> | |
| <tr><td>Graceful shutdown</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>SIGTERM + SIGINT handlers at <code>notion_sync_worker_main.py:100-106</code>. Uses <code>get_running_loop()</code> (better than Snowflake's deprecated <code>get_event_loop()</code>).</td></tr> | |
| <tr><td>Sync state management</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>Incremental via <code>last_modified_at</code> comparison. Exclusions in <code>resource_metadata</code>. <code>last_sync_at</code> set at sync start.</td></tr> | |
| <tr><td>Resource tracking</td><td><span class="badge badge-pass">PASS</span></td><td>Yes</td><td>Both <code>IntegrationResource</code> (page trees) and <code>IntegrationContent</code> (individual pages) used correctly.</td></tr> | |
| </table> | |
| <h3>Architectural Pattern Compliance</h3> | |
| <table> | |
| <tr><th>Pattern</th><th>Status</th><th>Notes</th></tr> | |
| <tr><td>Transformer performs substantive work</td><td><span class="badge badge-pass">PASS</span></td><td><code>blocks_to_text()</code> does genuine shape conversion: nested block tree → flat markdown. Verbs: extracts, formats, recurses, joins.</td></tr> | |
| <tr><td>Content/Material models have different shapes</td><td><span class="badge badge-pass">PASS</span></td><td><code>DocumentContent(data: str)</code> → <code>DocumentMaterial(data_bytes: bytes)</code>. str→bytes encoding + default resolution.</td></tr> | |
| <tr><td>HTTP client lifecycle matches reference</td><td><span class="badge badge-pass">PASS</span></td><td>Dedicated rate-limited client per sync, shared client for API routes, proper <code>async with</code> cleanup.</td></tr> | |
| <tr><td>Multi-component features E2E complete</td><td><span class="badge badge-partial">PARTIAL</span></td><td>All components wired, but sync_trigger not called from routes (<a href="#detail-H4">H4</a>).</td></tr> | |
| <tr><td>No duplicated core logic</td><td><span class="badge badge-fail">FAIL</span></td><td>Sync loop duplicated between <code>sync.py</code> and <code>notion_sync_service.py</code> (<a href="#detail-H1">H1</a>).</td></tr> | |
| </table> | |
| <div class="note"> | |
| <strong>Temporal config variations (documented, acceptable):</strong> Activity timeout 60min (vs Snowflake 30min) due to recursive block extraction at 3 req/s. Heartbeat timeout 5min (vs 2min). Max concurrent activities 5 (vs 10). All justified by Notion's tight rate limit. | |
| </div> | |
| <!-- ============================================================ --> | |
| <!-- CODE SMELLS --> | |
| <!-- ============================================================ --> | |
| <h2 id="code-smells">Semantic Clarity — Code Smells</h2> | |
| <div class="card" id="detail-M1"> | |
| <span class="badge badge-medium">Medium</span> | |
| <h4>M1: Magic timeout constants scattered across files</h4> | |
| <p><strong>Location:</strong> <code>client.py:94</code>, <code>connection.py:85</code>, <code>cli.py:120</code>, <code>notion_sync_worker_main.py:41</code>, <code>resource_service.py:42</code></p> | |
| <pre><code># client.py:94 | |
| http_client = httpx.AsyncClient(transport=transport, timeout=httpx.Timeout(60.0)) | |
| # connection.py:85 | |
| async with httpx.AsyncClient(timeout=httpx.Timeout(30.0)) as client: | |
| # cli.py:120 | |
| http_client = httpx.AsyncClient(timeout=httpx.Timeout(120.0)) | |
| # notion_sync_worker_main.py:41 | |
| http_client = httpx.AsyncClient(timeout=60.0) # bare float, not httpx.Timeout</code></pre> | |
| <p>Four different timeout values (30s, 60s, 60.0, 120s) across files with no named constants. The bare <code>timeout=60.0</code> in worker main is inconsistent with <code>httpx.Timeout(60.0)</code> elsewhere. Define named constants in <code>settings.py</code> and use <code>httpx.Timeout(...)</code> consistently.</p> | |
| </div> | |
| <div class="card" id="detail-M2"> | |
| <span class="badge badge-medium">Medium</span> | |
| <h4>M2: <code>extract_page_title()</code> returns <code>""</code> instead of <code>None</code></h4> | |
| <p><strong>Location:</strong> <code>transform.py:73</code></p> | |
| <pre><code>def extract_page_title(page: dict) -> str: | |
| for prop_data in page.get("properties", {}).values(): | |
| if prop_data.get("type") == "title": | |
| return "".join(rt.get("plain_text", "") for rt in prop_data.get("title", [])) | |
| return "" # sentinel masking absence</code></pre> | |
| <p>Callers have to check <code>title or "Untitled"</code> (seen at <code>resource_service.py:147</code>) but some don't, passing <code>""</code> through as a valid title. Change return type to <code>str | None</code> and return <code>None</code> when no title found, making the "no title" state explicit.</p> | |
| </div> | |
| <div class="card" id="detail-M3"> | |
| <span class="badge badge-medium">Medium</span> | |
| <h4>M3: Orphaned Secret Manager secrets on disconnect</h4> | |
| <p><strong>Location:</strong> <code>connection.py:268-270</code></p> | |
| <pre><code># NOTE: The Secret Manager secret is intentionally not deleted here. | |
| # The DB token record (which has the token_id referencing the secret) is deleted, | |
| # orphaning the secret. This is consistent with Snowflake and Looker connectors.</code></pre> | |
| <p>When a user disconnects, the Secret Manager secret is orphaned. The comment justifies this by citing consistency with other connectors. While consistent, orphaned secrets are a cost and security concern. Either add best-effort secret deletion (with error handling) or document as tracked tech debt with a ticket reference.</p> | |
| </div> | |
| <div class="card" id="detail-M4"> | |
| <span class="badge badge-medium">Medium</span> | |
| <h4>M4: Per-sync client creation prevents rate limiter sharing</h4> | |
| <p><strong>Location:</strong> <code>client.py:92-94</code></p> | |
| <pre><code>limiter = AsyncLimiter(RATE_LIMIT_PER_SECOND, 1) | |
| transport = RateLimitedTransport(httpx.AsyncHTTPTransport(), limiter) | |
| http_client = httpx.AsyncClient(transport=transport, timeout=httpx.Timeout(60.0))</code></pre> | |
| <p>Each <code>NotionClient.create()</code> builds a new rate limiter. Currently syncs are sequential so this is not a live bug, but if syncs for the same org ever become concurrent, they would collectively exceed Notion's per-integration 3 req/s limit (each thinking it has its own 3 req/s budget). Document the sequential requirement or extract client creation to share the limiter.</p> | |
| </div> | |
| <div class="card" id="detail-H2"> | |
| <span class="badge badge-high">High</span> | |
| <h4>H2: Missing <code>datetime.fromisoformat</code> error handling in Temporal sync path</h4> | |
| <p><strong>Location:</strong> <code>notion_sync_service.py:158</code></p> | |
| <pre><code># notion_sync_service.py:158 (NO error handling) | |
| page_edited_at = datetime.fromisoformat(last_edited).replace(tzinfo=None) | |
| # sync.py:132-135 (HAS error handling) | |
| try: | |
| page_edited_at = datetime.fromisoformat(last_edited).replace(tzinfo=None) | |
| except (ValueError, TypeError): | |
| logger.warning("Invalid last_edited_time for page %s: %r", page_id, last_edited)</code></pre> | |
| <p>The shared <code>sync.py</code> handles this defensively, but the Temporal path does not. An unexpected timestamp format from the Notion API would crash the entire sync for an org. This is a direct consequence of the code duplication in <a href="#detail-H1">H1</a>.</p> | |
| </div> | |
| <div class="card" id="detail-H3"> | |
| <span class="badge badge-high">High</span> | |
| <h4>H3: Incremental sync relies on minute-rounded timestamps</h4> | |
| <p><strong>Location:</strong> <code>notion_sync_service.py:156-161</code>, <code>sync.py:139</code></p> | |
| <pre><code>if page_edited_at <= existing.last_modified_at: | |
| pages_skipped += 1 | |
| continue</code></pre> | |
| <p>Notion's <code>last_edited_time</code> is <a href="https://developers.notion.com/changelog/last-edited-time-is-now-rounded-to-the-nearest-minute">rounded down to the nearest minute</a>. If content is synced and then edited within the same minute, the next sync sees the same (or earlier) timestamp and skips the change. Practical impact is low given hourly sync, but the edge case should be documented. Consider adding periodic full-sync (ignoring timestamps) as a safety net.</p> | |
| </div> | |
| <div class="card" id="detail-L1"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L1: Magic strings for sync event actions</h4> | |
| <p><strong>Location:</strong> <code>sync.py:34</code></p> | |
| <pre><code>@dataclass | |
| class PageSyncEvent: | |
| action: str # "synced", "skipped_excluded", "skipped_unchanged"</code></pre> | |
| <p>Consumers compare against string literals (<code>cli.py:142-147</code>). A typo would be a silent bug. Define a <code>PageSyncAction(StrEnum)</code>.</p> | |
| </div> | |
| <div class="card" id="detail-L2"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L2: Hardcoded <code>"success"</code> string instead of enum</h4> | |
| <p><strong>Location:</strong> <code>store.py:215</code></p> | |
| <pre><code>if knowledge_document_id: | |
| existing.ingestion_status = "success"</code></pre> | |
| <p>The codebase defines <code>IngestionStatus.SUCCESS = "success"</code>. Use <code>IngestionStatus.SUCCESS.value</code>.</p> | |
| </div> | |
| <div class="card" id="detail-L3"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L3: <code>get_sync_trigger</code> has wrong return type</h4> | |
| <p><strong>Location:</strong> <code>routes.py:52-54</code></p> | |
| <pre><code>def get_sync_trigger() -> None: | |
| """Get sync trigger service. Overridden by app factory.""" | |
| raise NotImplementedError("Must be overridden by app factory")</code></pre> | |
| <p>Return type should be <code>-> NotionSyncTrigger</code>, not <code>-> None</code>.</p> | |
| </div> | |
| <div class="card" id="detail-L4"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L4: Metrics port hardcoded</h4> | |
| <p><strong>Location:</strong> <code>notion_sync_worker_main.py:134</code></p> | |
| <pre><code>python_metrics_port = 8414 | |
| start_http_server(python_metrics_port, registry=REGISTRY)</code></pre> | |
| <p>Add <code>metrics_port: int = 8414</code> to <code>NotionTemporalSettings</code>.</p> | |
| </div> | |
| <div class="card" id="detail-L5"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L5: Notion URL hardcoded despite <code>settings.app_base_url</code></h4> | |
| <p><strong>Location:</strong> <code>store.py:60</code>, <code>temporal/models.py:41</code></p> | |
| <pre><code># store.py:60 | |
| "notion_url": f"https://www.notion.so/{page_id.replace('-', '')}", | |
| # temporal/models.py:41 | |
| return f"https://www.notion.so/{clean_id}"</code></pre> | |
| <p><code>settings.py:20</code> defines <code>app_base_url = "https://www.notion.so"</code> but neither file uses it. Either use the setting or remove it.</p> | |
| </div> | |
| <div class="card" id="detail-L6"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L6: f-string in <code>logger.info()</code></h4> | |
| <p><strong>Location:</strong> <code>notion_sync_worker_main.py:136</code></p> | |
| <pre><code>logger.info(f"Started Python prometheus metrics server on port {python_metrics_port}")</code></pre> | |
| <p>Use lazy <code>%</code>-style: <code>logger.info("Started Python prometheus metrics server on port %d", python_metrics_port)</code>.</p> | |
| </div> | |
| <div class="card" id="detail-L7"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L7: Custom UUID namespace diverges from codebase pattern</h4> | |
| <p><strong>Location:</strong> <code>temporal/models.py:9</code></p> | |
| <pre><code>_NOTION_NAMESPACE = uuid.UUID("a1c2e4f6-0000-4000-8000-000000000001")</code></pre> | |
| <p>Other connectors use <code>uuid.NAMESPACE_URL</code> or <code>uuid.NAMESPACE_DNS</code>. The fabricated UUID is functional but undocumented. Add a comment noting this namespace must never change without migrating existing document IDs.</p> | |
| </div> | |
| <div class="card" id="detail-L8"> | |
| <span class="badge badge-low">Low</span> | |
| <h4>L8: No 1Password entries for Notion</h4> | |
| <p><strong>Location:</strong> <code>src/app-frontend/.env.op</code></p> | |
| <p>No Notion entries found. Acceptable for an API-key integration where the token is stored in Secret Manager at connect-time (not OAuth client credentials). Noted for completeness.</p> | |
| </div> | |
| <!-- ============================================================ --> | |
| <!-- ASSUMPTION VALIDATION --> | |
| <!-- ============================================================ --> | |
| <h2 id="assumptions">Assumption Validation</h2> | |
| <div class="card" id="detail-C1-assumption"> | |
| <span class="badge badge-critical">Incorrect</span> | |
| <h4>Block container types are complete</h4> | |
| <p><strong>Location:</strong> <code>client.py:34</code></p> | |
| <p><strong>Assumption:</strong> Only <code>toggle</code>, <code>column</code>, <code>column_list</code>, and <code>synced_block</code> can contain <code>child_page</code> blocks.</p> | |
| <p><strong>Reality:</strong> The Notion API documents that <code>bulleted_list_item</code>, <code>callout</code>, <code>numbered_list_item</code>, <code>paragraph</code>, <code>quote</code>, <code>template</code>, <code>to_do</code>, and toggleable headings all support children and can contain child pages. See <a href="#detail-C1">C1</a>.</p> | |
| </div> | |
| <div class="card"> | |
| <span class="badge badge-pass">Correct</span> | |
| <h4>Rate limit: 3 requests per second</h4> | |
| <p><strong>Location:</strong> <code>client.py:29</code></p> | |
| <p>Notion API documentation confirms an average of 3 requests per second per integration. The <code>AsyncLimiter(3, 1)</code> is slightly more conservative than allowed (Notion permits some bursts) but appropriate for a production sync service.</p> | |
| </div> | |
| <div class="card"> | |
| <span class="badge badge-pass">Correct</span> | |
| <h4>Retry status codes: 429, 500, 502, 503, 504</h4> | |
| <p><strong>Location:</strong> <code>client.py:37-41</code></p> | |
| <p>Appropriate for Notion API. Minor optimization opportunity: parse <code>Retry-After</code> header on 429 responses instead of using exponential backoff starting at 4 seconds.</p> | |
| </div> | |
| <div class="card"> | |
| <span class="badge badge-pass">Correct</span> | |
| <h4><code>APIResponseError</code> has a <code>.status</code> attribute</h4> | |
| <p><strong>Location:</strong> <code>client.py:39</code>, <code>connection.py:91</code>, <code>resource_service.py:122</code></p> | |
| <p>Confirmed from notion-sdk-py source: <code>APIResponseError</code> inherits from <code>HTTPResponseError</code> which has <code>status: int</code>.</p> | |
| </div> | |
| <div class="card"> | |
| <span class="badge badge-partial">Partially correct</span> | |
| <h4>Incremental sync via <code>last_edited_time</code></h4> | |
| <p><strong>Location:</strong> <code>notion_sync_service.py:156-161</code></p> | |
| <p>Editing blocks within a page generally updates the page's <code>last_edited_time</code>, but the value is rounded to the nearest minute. Rapid edits within the same minute as a sync can be missed. See <a href="#detail-H3">H3</a>.</p> | |
| </div> | |
| <div class="card"> | |
| <span class="badge badge-pass">Non-issue</span> | |
| <h4>AsyncLimiter thread safety</h4> | |
| <p><strong>Location:</strong> <code>client.py:44-53</code></p> | |
| <p><code>AsyncLimiter</code> is async-only and used only within <code>async</code> methods. The asyncio event loop provides serialization. Correct usage.</p> | |
| </div> | |
| <!-- ============================================================ --> | |
| <!-- TEST COVERAGE --> | |
| <!-- ============================================================ --> | |
| <h2 id="test-coverage">Test Coverage Analysis</h2> | |
| <h3>Strengths</h3> | |
| <ul> | |
| <li><strong>Core business logic is well covered:</strong> Sync service tests verify full sync, incremental sync, error handling, and new page discovery.</li> | |
| <li><strong>Good edge cases in schemas:</strong> URL parsing handles full URLs, raw UUIDs, raw hex, query params, whitespace, and invalid inputs.</li> | |
| <li><strong>Transform tests are thorough:</strong> All major block types tested with clear expected output.</li> | |
| <li><strong>Client tests cover recursive crawling:</strong> Single-page, nested, and toggle-wrapped child page scenarios.</li> | |
| <li><strong>Connection tests verify important business rules:</strong> Creator info preservation on reconnect, idempotent disconnect.</li> | |
| <li><strong>Exclusion workflow covered end-to-end:</strong> Adding, removing, idempotent, and empty-metadata edge cases.</li> | |
| <li><strong>Appropriate mock granularity:</strong> Tests generally mock at service boundaries, not implementation details.</li> | |
| </ul> | |
| <h3>Priority Gaps</h3> | |
| <table> | |
| <tr><th>Gap</th><th>Severity</th><th>Untested Path</th></tr> | |
| <tr id="detail-H5"><td>Shared sync loop (<code>sync.py</code>) — 207 lines of core crawl/filter/extract/ingest logic with zero direct tests</td><td><span class="badge badge-high">High</span></td><td><code>sync.py:sync_page_tree()</code> lines 55-207</td></tr> | |
| <tr id="detail-H6"><td>Temporal workflows and activities — no tests for heartbeat logic, error re-raise, retry policy, orchestration</td><td><span class="badge badge-high">High</span></td><td><code>run_notion_sync_workflow.py</code>, <code>trigger_notion_sync_workflow.py</code></td></tr> | |
| <tr id="detail-H7"><td>Routes module — 8 HTTP endpoints with no tests for request parsing, error-to-HTTPException mapping, DI wiring</td><td><span class="badge badge-high">High</span></td><td><code>routes.py</code> lines 75-214</td></tr> | |
| <tr id="detail-M5"><td><code>update_page()</code> resource service method — user-facing PATCH endpoint</td><td><span class="badge badge-medium">Medium</span></td><td><code>resource_service.py:update_page()</code> lines 186-217</td></tr> | |
| <tr id="detail-M6"><td>App factory <code>create_notion_app()</code> — dependency override wiring</td><td><span class="badge badge-medium">Medium</span></td><td><code>app.py</code> lines 20-62</td></tr> | |
| <tr id="detail-M7"><td>Connection error/rollback paths — Secret Manager failure, DB commit failure</td><td><span class="badge badge-medium">Medium</span></td><td><code>connection.py</code> lines 101-110, 143-151, 203-210</td></tr> | |
| <tr id="detail-M8"><td>CLI <code>sync</code> command — largest function in <code>cli.py</code>, 100+ lines</td><td><span class="badge badge-medium">Medium</span></td><td><code>cli.py:_sync()</code> lines 91-194</td></tr> | |
| </table> | |
| <details> | |
| <summary>Expand full list of untested code paths</summary> | |
| <ul> | |
| <li><code>sync.py:sync_page_tree()</code> — entire shared sync loop (exclusion filtering, incremental detection, content extraction, ingest callback, progress reporting)</li> | |
| <li><code>run_notion_sync_workflow.py:RunNotionSyncActivity.run_sync()</code> — heartbeat task, error logging/re-raise, heartbeat cancellation in finally block</li> | |
| <li><code>run_notion_sync_workflow.py:RunNotionSyncWorkflow.run()</code> — retry policy, timeout config</li> | |
| <li><code>trigger_notion_sync_workflow.py:TriggerNotionSyncActivity</code> — list_syncs delegation, trigger_sync workflow start, WorkflowAlreadyStartedError handling</li> | |
| <li><code>trigger_notion_sync_workflow.py:TriggerNotionSyncWorkflow.run()</code> — orchestration over discovered syncs</li> | |
| <li><code>sync_trigger.py:NotionSyncTrigger.sync_resource()</code> — fire-and-forget pattern with exception suppression</li> | |
| <li><code>notion_sync_worker_main.py</code> — worker construction, schedule creation, signal handling</li> | |
| <li><code>routes.py</code> — all 8 HTTP endpoint handlers</li> | |
| <li><code>app.py:create_notion_app()</code> — dependency override wiring</li> | |
| <li><code>resource_service.py:update_page()</code> — toggle enable/disable</li> | |
| <li><code>resource_service.py:_delete_knowledge_documents()</code> — Briefs HTTP delete cascade</li> | |
| <li><code>cli.py:_sync()</code> — local sync with ingest callback, dry-run mode, error handling</li> | |
| <li><code>client.py:NotionClient.create()</code> success path — RateLimitedTransport wiring</li> | |
| <li><code>client.py:RateLimitedTransport</code> — rate limiter behavior</li> | |
| <li><code>connection.py</code> error paths — Secret Manager failure, DB rollback, generic exception handling</li> | |
| <li><code>store.py:update_resource_sync_time()</code></li> | |
| <li><code>metrics.py</code> — Prometheus metric recording</li> | |
| <li><code>transform.py</code> — <code>numbered_list_item</code> formatting, <code>child_database</code> type</li> | |
| </ul> | |
| </details> | |
| <!-- ============================================================ --> | |
| <!-- DEPENDENCIES --> | |
| <!-- ============================================================ --> | |
| <h2 id="dependencies">Dependencies</h2> | |
| <div class="card"> | |
| <table> | |
| <tr><th>Package</th><th>Specified</th><th>Latest</th><th>Status</th></tr> | |
| <tr><td><code>notion-client</code></td><td><code>>=2.7.0</code></td><td>2.7.0</td><td><span class="badge badge-pass">Current</span></td></tr> | |
| <tr><td><code>aiolimiter</code></td><td><code>>=1.1.0</code></td><td>1.2.1</td><td><span class="badge badge-pass">OK</span> — open-ended constraint permits latest</td></tr> | |
| </table> | |
| <p>Both Notion-specific dependencies are current. No security concerns or version gaps detected.</p> | |
| </div> | |
| </body> | |
| </html> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment