feat: add DB-mode property support (read & write)#24
feat: add DB-mode property support (read & write)#24arndjan wants to merge 5 commits intoergut:mainfrom
Conversation
Logseq's DB-mode stores class properties as Datascript entities (:user.property/* idents) rather than inline markdown key:: value pairs. This means get_page_content returned no properties for DB-mode graphs. Changes: - Add datascript_query() method for raw Datalog queries - Add get_block_db_properties() to read class properties via Datascript - Extend get_page_content to render DB-mode properties alongside markdown-mode properties in key:: value format - Add set_block_properties tool to write DB-mode properties by display name, resolving to internal :user.property/* idents automatically Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewHi @arndjan — thanks for this PR! The feature itself is genuinely useful and I can see you've tested it against a real DB-mode graph. Before merging, I'd like to request a few changes to make sure existing Markdown-mode users aren't affected. Context: DB mode is still in betaAs of March 2026, Logseq DB mode is still opt-in and in beta — the Logseq team explicitly recommends using DB graphs for testing only, as data loss is possible. The project is on a path toward v2.0.0 (DB-only), but a stable release before Q3 2026 is uncertain. Given that the vast majority of current users of this MCP server are on Markdown/file-based graphs, we need to make sure this PR does not affect them in any way — either in performance or output correctness. That said, we're not rejecting this because of DB mode's beta status. Logseq has been clear that v2.0.0 is the future — file-based graphs are in maintenance mode with no new features planned. Being early with DB support is valuable, and the API calls this PR uses ( Required changes1. Gate DB-mode queries behind a feature flag (critical)Right now, Please add an opt-in mechanism. The simplest approach is an environment variable: # In logseq.py or tools.py
import os
LOGSEQ_DB_MODE = os.getenv("LOGSEQ_DB_MODE", "false").lower() == "true"Then in if LOGSEQ_DB_MODE:
try:
db_properties = api.get_blocks_db_properties(blocks)
except Exception as e:
logger.warning(f"Could not fetch DB-mode properties: {e}")This means Markdown-mode users see zero change in behavior or performance. 2. Fix potential double-rendering of Markdown-mode propertiesThe new property rendering loop in If the intent is only to support DB-mode properties (which are not in # Only render db_properties if in DB mode — markdown properties are already in content
if db_properties and block_uuid in db_properties:
for key, value in db_properties[block_uuid].items():
lines.append(f"{indent} {key}:: {value}")3. Address the N+1 query problem
Consider batching this with a single Datascript query that fetches all [:find ?block ?attr ?val
:where
[?block ?attr ?val]
[(clojure.string/starts-with? (str ?attr) ":user.property/")]]Even a partial improvement (e.g. resolving all property names in one pass) would significantly reduce the API call count. 4. Add testsNo tests were added for the new
Minor notes
SummaryThe feature fills a real gap for DB-mode users and the implementation logic is sound. The main blocker is the unconditional Datascript querying that runs for all users regardless of mode. Once there's a flag to opt in, the double-render risk is addressed, and tests are added, I'd be happy to merge this. Thanks for the detailed PR description — the "how it works" section made review much easier. |
Addresses code review feedback on PR ergut#24: all DB-mode property queries and rendering are now gated behind LOGSEQ_DB_MODE=true. Markdown-mode users see zero change in behavior or performance. - Add _db_mode flag parsed from LOGSEQ_DB_MODE env var - Guard get_blocks_db_properties call in get_page_content - Guard property rendering in _format_block_tree (fixes double-render) - Add early return in SetBlockPropertiesToolHandler when not in DB-mode Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
17 tests covering: - get_block_db_properties: happy path, no properties, API failure, string values - get_blocks_db_properties: recursive tree processing, empty input - resolve_property_ident: found, case-insensitive, not found, API failure - _format_block_tree: DB properties rendering, flag gating, no duplicates, internal property filtering, nested indentation - Feature flag integration: get_page_content skips DB queries without flag, set_block_properties blocked without flag Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refactors get_blocks_db_properties to use batched queries: - Phase 1: per-block attribute query (1 call per block, unchanged) - Phase 2: batch resolve all :user.property/* idents (1 call) - Phase 3: batch resolve all entity titles (1 call) For 20 blocks with 3 properties each, reduces from ~200 HTTP round-trips to ~22. Both batch methods include fallback to individual queries if the batched datascript query fails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove set_block_db_property (duplicate of _upsert_block_property), SetBlockPropertiesToolHandler now uses _upsert_block_property directly - Improve datascript_query docstring with return shape documentation - Fix integration test handler count (15 → 16 for set_block_properties) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Hey Salih, thanks for the thorough review — really appreciate you taking the time to lay out all the details so clearly. Decided to put in some extra effort this Friday evening and worked through all your feedback points. Here's what I've done across four commits: Feature flag ( Double-rendering fix — The Tests — Added 18 unit tests covering N+1 batching — Refactored Cleanup — Removed the duplicate Glad to make this small contribution to the project — mcp-logseq has been really useful for my workflow. Let me know if there's anything else you'd like me to adjust! |
Summary
Logseq's DB-mode stores class/tag properties as Datascript entities (
:user.property/*idents) rather than inline markdownkey:: valuepairs. This meansget_page_contentcurrently returns no properties for DB-mode graphs.This PR adds:
get_page_contentnow queries Datascript for:user.property/*attributes on each block and renders them askey:: valuelines, so DB-mode properties show up alongside markdown-mode propertiesset_block_propertiestool that accepts human-readable property names (e.g. "Content status"), resolves them to internal idents, and callsupsertBlockPropertyChanges
logseq.py: Adddatascript_query(),get_block_db_properties(),get_blocks_db_properties(),set_block_db_property(),resolve_property_ident()and helper methodstools.py: Extend_format_block_tree()to render DB-mode properties; addSetBlockPropertiesToolHandlerserver.py: Registerset_block_propertiestool handlerHow it works
Reading: After fetching the block tree, we query each block's entity for attributes starting with
:user.property/. For each, we resolve the property's display name (via entity title lookup) and the value's display name (entity reference → title). Results are rendered askey:: valuelines in the output.Writing: The
set_block_propertiestool takes{display_name: value}pairs, scans all:db/idententries for:user.property/*matches, compares entity titles case-insensitively, then callsupsertBlockPropertywith the resolved ident.Test plan
get_page_contentshows properties likeContent status:: kiem,Topics:: waarden veranderingset_block_propertiesresolves display names and updates values🤖 Generated with Claude Code