From cb68930ec8acefa8e00b26e848761f0d575a6837 Mon Sep 17 00:00:00 2001 From: rcmerci Date: Thu, 26 Mar 2026 14:19:44 +0800 Subject: [PATCH] 066-logseq-cli-list-property-cardinality-column.md --- cli-e2e/spec/non_sync_cases.edn | 3 +- ...eq-cli-list-property-cardinality-column.md | 182 ++++++++++++++++++ docs/cli/logseq-cli.md | 10 +- src/main/logseq/cli/command/list.cljs | 1 + src/main/logseq/cli/common/db_worker.cljs | 9 +- src/main/logseq/cli/format.cljs | 29 ++- src/test/logseq/cli/commands_test.cljs | 36 +++- .../logseq/cli/common/db_worker_test.cljs | 14 +- src/test/logseq/cli/format_test.cljs | 30 ++- 9 files changed, 297 insertions(+), 17 deletions(-) create mode 100644 docs/agent-guide/066-logseq-cli-list-property-cardinality-column.md diff --git a/cli-e2e/spec/non_sync_cases.edn b/cli-e2e/spec/non_sync_cases.edn index 2f771a3268..fce5e204f4 100644 --- a/cli-e2e/spec/non_sync_cases.edn +++ b/cli-e2e/spec/non_sync_cases.edn @@ -163,7 +163,8 @@ :expect {:exit 0 :stdout-json-paths {[:status] "ok" [:data :items 0 :title] "score" - [:data :items 0 :type] "number"}} + [:data :items 0 :type] "number" + [:data :items 0 :cardinality] "one"}} :covers {:commands ["upsert property" "list property"] :options {:global ["--config" "--graph" "--data-dir" "--output"] :upsert ["--name" "--type" "--cardinality"] diff --git a/docs/agent-guide/066-logseq-cli-list-property-cardinality-column.md b/docs/agent-guide/066-logseq-cli-list-property-cardinality-column.md new file mode 100644 index 0000000000..05c673f7af --- /dev/null +++ b/docs/agent-guide/066-logseq-cli-list-property-cardinality-column.md @@ -0,0 +1,182 @@ +# Logseq CLI List Property Cardinality Column Plan + +Goal: Add a `CARDINALITY` column to `logseq list property` output, aligned with the existing `TYPE` column behavior and powered by current `logseq-cli -> db-worker-node` thread-api flow. + +Architecture: Keep the transport contract unchanged (`:thread-api/cli-list-properties`) and extend the property list payload shape plus human formatter columns. + +Tech Stack: ClojureScript, Datascript, Promesa, `logseq.cli.command.list`, `logseq.cli.common.db-worker`, `frontend.worker.db-core`, `logseq.cli.format`. + +Related: +- `docs/agent-guide/045-logseq-cli-property-type-and-upsert-option-unification.md` +- `docs/agent-guide/064-logseq-cli-integration-test-shell-refactor.md` +- `docs/cli/logseq-cli.md` + +## Problem Statement + +`list property` currently includes `TYPE` in default output, but it does not show property cardinality in human tables. + +In current implementation: +- CLI list execution (`src/main/logseq/cli/command/list.cljs`) already supports property-specific fields and formatting hooks. +- db-worker thread API (`src/main/frontend/worker/db_core.cljs`) already routes `:thread-api/cli-list-properties` to `logseq.cli.common.db-worker/list-properties`. +- Property list payload currently includes `:logseq.property/type` in non-expanded mode, but does not guarantee cardinality is present for all properties. +- Human formatter (`src/main/logseq/cli/format.cljs`) has no `CARDINALITY` column for `:list-property`. + +Users cannot quickly inspect whether a property is `one` or `many` from the list table. + +## Current Data Flow (Baseline) + +```text +logseq list property + -> src/main/logseq/cli/command/list.cljs execute-list-property + -> transport/invoke :thread-api/cli-list-properties + -> db-worker-node /v1/invoke forwarding + -> src/main/frontend/worker/db_core.cljs :thread-api/cli-list-properties + -> src/main/logseq/cli/common/db_worker.cljs list-properties + -> back to CLI formatter src/main/logseq/cli/format.cljs +``` + +This flow is already correct and should be preserved. The change is additive in payload fields and table rendering. + +## Target Contract + +1. Human output for `list property` includes a `CARDINALITY` column by default. +2. Cardinality values render as `one` or `many` (not raw Datascript keywords). +3. If a property has no explicit `:db/cardinality`, treat it as `one` (Datascript default semantics). +4. For `--output json`, property items expose cardinality via `cardinality` (string `"one"` or `"many"`). +5. For `--output edn`, property items expose cardinality via `:db/cardinality`. +6. `--fields` and `--sort` for `list property` can reference `cardinality`. +7. Existing thread-api method names and db-worker-node invoke protocol remain unchanged. + +## Design Decisions + +### 1) Cardinality source of truth + +Use `:db/cardinality` from property entity returned by Datascript. + +Normalization rule: +- `:db.cardinality/many` => `many` +- `:db.cardinality/one` or missing value => `one` + +### 2) Payload layer behavior + +In `src/main/logseq/cli/common/db_worker.cljs` `list-properties`: +- Ensure non-expanded list items always include `:db/cardinality`. +- For entities missing explicit cardinality, populate `:db/cardinality :db.cardinality/one`. +- Keep existing behavior for `:logseq.property/type`, classes, description, and built-in filtering. + +### 3) CLI list command behavior + +In `src/main/logseq/cli/command/list.cljs`: +- Extend `list-property-field-map` with `"cardinality" -> :db/cardinality`. +- Keep existing default option behavior (`--with-type` remains unchanged). +- Reuse existing `apply-sort` / `apply-fields` pipeline so cardinality works with no new execution path. + +### 4) Human formatter behavior + +In `src/main/logseq/cli/format.cljs`: +- Add a cardinality normalization helper (similar style to `normalize-property-type`). +- Add `CARDINALITY` to `list-property-columns` near `TYPE`. +- Render `one` / `many`, and fallback to `-` only for truly unknown values. + +### 5) Structured output key behavior + +- Keep payload source key as `:db/cardinality` in command/data layer. +- In JSON formatting path, map property-list item key `db/cardinality` to `cardinality`. +- EDN output keeps `:db/cardinality` unchanged. + +## Implementation Plan (TDD Order) + +1. Add failing formatter tests in `src/test/logseq/cli/format_test.cljs`: + - `list property` table includes `CARDINALITY` header. + - `:db.cardinality/many` renders `many`. + - missing/`one` cardinality renders `one`. + +2. Add failing db-worker payload tests in `src/test/logseq/cli/common/db_worker_test.cljs`: + - non-expanded `list-properties` includes `:db/cardinality` key. + - custom property created without explicit cardinality still yields `:db.cardinality/one`. + +3. Add failing output-format tests in `src/test/logseq/cli/format_test.cljs`: + - JSON output for `list property` uses `cardinality` key. + - EDN output for `list property` keeps `:db/cardinality`. + +4. Add failing command parsing/execution tests in `src/test/logseq/cli/commands_test.cljs`: + - `list property --fields name,type,cardinality` parses successfully. + - `list property --sort cardinality` is accepted and sorted through existing pipeline. + +5. Implement payload update in `src/main/logseq/cli/common/db_worker.cljs`. + +6. Implement field-map update in `src/main/logseq/cli/command/list.cljs`. + +7. Implement formatter updates in `src/main/logseq/cli/format.cljs`: + - human `CARDINALITY` column + value normalization. + - JSON key remap `db/cardinality` -> `cardinality` for `:list-property`. + +8. Update CLI docs in `docs/cli/logseq-cli.md`: + - document `CARDINALITY` as default `list property` column. + - document JSON key `cardinality` and EDN key `:db/cardinality`. + - mention default semantics (`one` when omitted in schema). + +9. Update shell e2e spec in `cli-e2e/spec/non_sync_cases.edn`: + - extend `property-upsert-and-list-json` expectation to include `cardinality` field/value. + +10. Run focused tests, then broader regression suite. + +## Files to Touch + +- `src/main/logseq/cli/common/db_worker.cljs` + - include normalized cardinality in property list payload. + +- `src/main/logseq/cli/command/list.cljs` + - add `cardinality` field mapping for sort/fields. + +- `src/main/logseq/cli/format.cljs` + - add `CARDINALITY` column and render normalization. + +- `src/test/logseq/cli/common/db_worker_test.cljs` + - contract tests for property list payload cardinality. + +- `src/test/logseq/cli/commands_test.cljs` + - parser/sort/fields coverage for new property field. + +- `src/test/logseq/cli/format_test.cljs` + - human table assertions with cardinality column. + +- `cli-e2e/spec/non_sync_cases.edn` + - structured output expectation coverage. + +- `docs/cli/logseq-cli.md` + - user-facing command/output reference update. + +## Verification Commands + +| Command | Expected | +| --- | --- | +| `bb dev:test -v logseq.cli.format-test/test-human-output-list-tag-property` | Property table assertions pass with `CARDINALITY`. | +| `bb dev:test -v logseq.cli.common.db-worker-test` | Non-expanded property payload includes cardinality contract. | +| `bb dev:test -v logseq.cli.commands-test/test-list-subcommand-parse` | `cardinality` fields/sort parsing passes. | +| `bb -f cli-e2e/bb.edn test --skip-build` | Non-sync CLI e2e list/property cases pass. | +| `bb dev:lint-and-test` | Full lint/test suite remains green. | + +## Risks and Mitigation + +- Risk: Some properties may not explicitly store `:db/cardinality`, causing missing-column behavior. + - Mitigation: Normalize missing to `:db.cardinality/one` in payload layer. + +- Risk: Human output snapshot expectations may break due to added column width/layout. + - Mitigation: Update formatter tests with explicit expected tables. + +- Risk: Structured output consumers may rely on previous key set. + - Mitigation: This is additive only; do not remove or rename existing fields. + +## Out of Scope + +- Adding a new CLI option such as `--with-cardinality`. +- Changing thread-api names or db-worker-node HTTP protocol. +- Changing `upsert property` cardinality semantics. + +## Decision + +- JSON output uses `cardinality`. +- EDN output uses `:db/cardinality`. + +Implementation note: keep command/data layer on `:db/cardinality`, and apply JSON-only key remapping in formatting path for `:list-property`. diff --git a/docs/cli/logseq-cli.md b/docs/cli/logseq-cli.md index 618de01cfd..abe7c750fc 100644 --- a/docs/cli/logseq-cli.md +++ b/docs/cli/logseq-cli.md @@ -178,7 +178,7 @@ Sync config persistence: Inspect and edit commands: - `list page [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list pages (defaults to `--sort updated-at`) - `list tag [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list tags (defaults to `--sort updated-at`) -- `list property [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list properties (defaults to `--sort updated-at`; `TYPE` is included by default even without `--expand`) +- `list property [--expand] [--limit ] [--offset ] [--sort ] [--order asc|desc]` - list properties (defaults to `--sort updated-at`; `TYPE` and `CARDINALITY` are included by default even without `--expand`; missing schema cardinality is treated as `one`) - `upsert block --content [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - create blocks; defaults to today’s journal page if no target is given - `upsert block --blocks [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - insert blocks via EDN vector - `upsert block --blocks-file [--target-page |--target-id |--target-uuid ] [--pos first-child|last-child|sibling]` - insert blocks from an EDN file @@ -232,9 +232,13 @@ Revision: Output formats: - Global `--output ` applies to all commands - Output formatting is controlled via global `--output`, `:output-format` in config, or `LOGSEQ_CLI_OUTPUT`. -- Human output is plain text. List/search commands render tables with a final `Count: N` line. For list and search subcommands, the ID column uses `:db/id` (not UUID). If `:db/ident` exists, an `IDENT` column is included. `list property` includes a dedicated `TYPE` column. Search table columns are `ID` and `TITLE`. Block titles can include multiple lines; multi-line rows align additional lines under the `TITLE` column. Times such as list `UPDATED-AT`/`CREATED-AT` and `graph info` `Created at` are shown in human-friendly relative form. Errors include error codes and may include a `Hint:` line. Use `--output json|edn` for structured output. +- Human output is plain text. List/search commands render tables with a final `Count: N` line. For list and search subcommands, the ID column uses `:db/id` (not UUID). If `:db/ident` exists, an `IDENT` column is included. `list property` includes dedicated `TYPE` and `CARDINALITY` columns. Search table columns are `ID` and `TITLE`. Block titles can include multiple lines; multi-line rows align additional lines under the `TITLE` column. Times such as list `UPDATED-AT`/`CREATED-AT` and `graph info` `Created at` are shown in human-friendly relative form. Errors include error codes and may include a `Hint:` line. Use `--output json|edn` for structured output. - `sync download` progress lines are streamed to stdout only when progress is enabled. In `json`/`edn` mode, progress is disabled by default unless `--progress true` is provided. -- For `list property`, `TYPE` is returned in default output (without `--expand`) for human and structured (`json`/`edn`) formats. +- For `list property`, `TYPE` and cardinality are returned in default output (without `--expand`) for human and structured (`json`/`edn`) formats. + - Human output renders cardinality as `one` or `many` in the `CARDINALITY` column. + - JSON uses `cardinality` with values `"one"` or `"many"`. + - EDN keeps `:db/cardinality` (for example `:db.cardinality/one` or `:db.cardinality/many`). + - When a property omits schema cardinality, CLI treats it as default `one`. - `upsert page` and `upsert block` return entity ids in `data.result` for JSON/EDN output, and include ids in human output. - Human example: ```text diff --git a/src/main/logseq/cli/command/list.cljs b/src/main/logseq/cli/command/list.cljs index daf1c68879..73a5ae6bfa 100644 --- a/src/main/logseq/cli/command/list.cljs +++ b/src/main/logseq/cli/command/list.cljs @@ -87,6 +87,7 @@ "updated-at" :block/updated-at "classes" :logseq.property/classes "type" :logseq.property/type + "cardinality" :db/cardinality "description" :logseq.property/description}) (def ^:private list-property-spec diff --git a/src/main/logseq/cli/common/db_worker.cljs b/src/main/logseq/cli/common/db_worker.cljs index 5aba3c1b79..1c00e4b9d7 100644 --- a/src/main/logseq/cli/common/db_worker.cljs +++ b/src/main/logseq/cli/common/db_worker.cljs @@ -14,6 +14,10 @@ (:db/ident e) (assoc :db/ident (:db/ident e)) (:logseq.property/type e) (assoc :logseq.property/type (:logseq.property/type e)))) +(defn- normalize-cardinality + [value] + (or value :db.cardinality/one)) + (defn list-properties "List properties for CLI" [db {:keys [expand include-built-in] :as options}] @@ -36,9 +40,10 @@ (update :logseq.property/classes #(mapv :db/ident %)) (:logseq.property/description e) (update :logseq.property/description db-property/property-value-content)) - ;; Keep property type in default list output (without --expand). + ;; Keep property type and cardinality in default list output (without --expand). (assoc (minimal-list-item e) - :logseq.property/type (:logseq.property/type e)))))))) + :logseq.property/type (:logseq.property/type e) + :db/cardinality (normalize-cardinality (:db/cardinality e))))))))) (defn list-tags "List tags for CLI" diff --git a/src/main/logseq/cli/format.cljs b/src/main/logseq/cli/format.cljs index 7c6bbd3405..a79ff667ae 100644 --- a/src/main/logseq/cli/format.cljs +++ b/src/main/logseq/cli/format.cljs @@ -14,13 +14,39 @@ entry)) value)) +(defn- normalize-property-cardinality + [value] + (cond + (nil? value) "one" + (keyword? value) (let [normalized (name value)] + (if (#{"one" "many"} normalized) + normalized + "-")) + (string? value) (if (#{"one" "many"} value) value "-") + :else "-")) + +(defn- remap-list-property-json-item + [item] + (if (and (map? item) (contains? item :db/cardinality)) + (-> item + (assoc :cardinality (normalize-property-cardinality (:db/cardinality item))) + (dissoc :db/cardinality)) + item)) + +(defn- normalize-json-data + [command data] + (if (and (= command :list-property) (map? data)) + (update data :items (fn [items] + (mapv remap-list-property-json-item (or items [])))) + data)) + (defn- ->json [{:keys [status data error command]}] (let [obj (js-obj)] (set! (.-status obj) (name status)) (cond (= status :ok) - (set! (.-data obj) (clj->js (normalize-json data))) + (set! (.-data obj) (clj->js (normalize-json (normalize-json-data command data)))) (= status :error) (do @@ -215,6 +241,7 @@ [["ID" (fn [item _] (or (:db/id item) (:id item))) [:db/id :id]] ["TITLE" (fn [item _] (or (:title item) (:block/title item) (:name item))) [:title :block/title :name]] ["TYPE" (fn [item _] (normalize-property-type (:logseq.property/type item))) [:logseq.property/type]] + ["CARDINALITY" (fn [item _] (normalize-property-cardinality (:db/cardinality item))) [:db/cardinality]] ["CLASSES" (fn [item _] (format-classes (:logseq.property/classes item))) [:logseq.property/classes]] ["UUID" (fn [item _] (or (:block/uuid item) "-")) [:block/uuid]] ["IDENT" (fn [item _] (:db/ident item)) [:db/ident]] diff --git a/src/test/logseq/cli/commands_test.cljs b/src/test/logseq/cli/commands_test.cljs index 771105802e..23cff60665 100644 --- a/src/test/logseq/cli/commands_test.cljs +++ b/src/test/logseq/cli/commands_test.cljs @@ -932,14 +932,16 @@ "--include-built-in" "--with-classes" "--with-type" - "--fields" "name,type"])] + "--sort" "cardinality" + "--fields" "name,type,cardinality"])] (is (true? (:ok? result))) (is (= :list-property (:command result))) (is (true? (get-in result [:options :expand]))) (is (true? (get-in result [:options :include-built-in]))) (is (true? (get-in result [:options :with-classes]))) (is (true? (get-in result [:options :with-type]))) - (is (= "name,type" (get-in result [:options :fields])))))) + (is (= "cardinality" (get-in result [:options :sort]))) + (is (= "name,type,cardinality" (get-in result [:options :fields])))))) (deftest test-list-subcommand-validation (testing "list page rejects mutually exclusive journal flags" @@ -1006,6 +1008,36 @@ (is false (str "unexpected error: " e)))) (p/finally done)))) +(deftest test-list-property-execute-supports-cardinality-sort-and-fields + (async done + (-> (p/with-redefs [cli-server/ensure-server! (fn [_ _] {:base-url "http://example"}) + transport/invoke (fn [_ method _ _] + (case method + :thread-api/cli-list-properties [{:db/id 30 + :block/title "Gamma" + :db/cardinality :db.cardinality/one} + {:db/id 10 + :block/title "Alpha" + :db/cardinality :db.cardinality/many} + {:db/id 20 + :block/title "Beta" + :db/cardinality :db.cardinality/many}] + (throw (ex-info "unexpected invoke" {:method method}))))] + (p/let [result (list-command/execute-list-property + {:repo "demo" + :options {:sort "cardinality" + :fields "id,title,cardinality"}} + {}) + items (get-in result [:data :items])] + (is (= [10 20 30] (mapv :db/id items))) + (is (= [#{:db/id :block/title :db/cardinality} + #{:db/id :block/title :db/cardinality} + #{:db/id :block/title :db/cardinality}] + (mapv (comp set keys) items))))) + (p/catch (fn [e] + (is false (str "unexpected error: " e)))) + (p/finally done)))) + (deftest test-verb-subcommand-parse-upsert-remove (testing "remove block parses with id" (let [result (commands/parse-args ["remove" "block" "--id" "10"])] diff --git a/src/test/logseq/cli/common/db_worker_test.cljs b/src/test/logseq/cli/common/db_worker_test.cljs index 21d7acc841..a225e87872 100644 --- a/src/test/logseq/cli/common/db_worker_test.cljs +++ b/src/test/logseq/cli/common/db_worker_test.cljs @@ -48,7 +48,8 @@ (deftest test-list-non-expanded-contract (let [db (create-test-db) required-keys #{:db/id :block/title :block/created-at :block/updated-at} - required-property-keys #{:db/id :block/title :block/created-at :block/updated-at :logseq.property/type} + required-property-keys #{:db/id :block/title :block/created-at :block/updated-at + :logseq.property/type :db/cardinality} visible-page (some #(when (= "Visible Page" (:block/title %)) %) (cli-db-worker/list-pages db {})) custom-tag-entity (first-user-tag-entity db) @@ -67,10 +68,19 @@ (is (some? custom-tag)) (is (set/subset? required-keys (set (keys custom-tag))))) - (testing "list-properties non-expanded includes stable id and timestamps" + (testing "list-properties non-expanded includes stable id, timestamps, and cardinality" (is (some? custom-property)) (is (set/subset? required-property-keys (set (keys custom-property))))))) +(deftest test-list-properties-default-cardinality-contract + (let [db (create-test-db) + custom-property-title (:block/title (first-user-property-entity db)) + custom-property (some #(when (= custom-property-title (:block/title %)) %) + (cli-db-worker/list-properties db {}))] + (testing "property without explicit cardinality defaults to :db.cardinality/one" + (is (some? custom-property)) + (is (= :db.cardinality/one (:db/cardinality custom-property)))))) + (deftest test-list-tags-and-properties-include-built-in-default (let [db (create-test-db) built-in-tag-ids (->> (d/datoms db :avet :block/tags :logseq.class/Tag) diff --git a/src/test/logseq/cli/format_test.cljs b/src/test/logseq/cli/format_test.cljs index 1cac14a926..8e8371d3c3 100644 --- a/src/test/logseq/cli/format_test.cljs +++ b/src/test/logseq/cli/format_test.cljs @@ -185,21 +185,23 @@ :data {:items [{:block/title "Prop" :db/id 99 :logseq.property/type :node + :db/cardinality :db.cardinality/many :block/created-at 40000 :block/updated-at 90000}]}} {:output-format nil :now-ms 100000})] - (is (= (str "ID TITLE TYPE UPDATED-AT CREATED-AT\n" - "99 Prop node 10s ago 1m ago\n" + (is (= (str "ID TITLE TYPE CARDINALITY UPDATED-AT CREATED-AT\n" + "99 Prop node many 10s ago 1m ago\n" "Count: 1") result)))) - (testing "list property renders missing type as -" + (testing "list property renders missing type as - and missing cardinality as one" (let [result (format/format-result {:status :ok :command :list-property :data {:items [{:block/title "Prop" :db/id 99 :logseq.property/type :node + :db/cardinality :db.cardinality/many :block/created-at 40000 :block/updated-at 90000} {:block/title "Untyped" @@ -208,12 +210,28 @@ :block/updated-at 90000}]}} {:output-format nil :now-ms 100000})] - (is (= (str "ID TITLE TYPE UPDATED-AT CREATED-AT\n" - "99 Prop node 10s ago 1m ago\n" - "100 Untyped - 10s ago 1m ago\n" + (is (= (str "ID TITLE TYPE CARDINALITY UPDATED-AT CREATED-AT\n" + "99 Prop node many 10s ago 1m ago\n" + "100 Untyped - one 10s ago 1m ago\n" "Count: 2") result))))) +(deftest test-list-property-json-edn-cardinality-shape + (testing "list property json uses cardinality key while edn keeps :db/cardinality" + (let [base-result {:status :ok + :command :list-property + :data {:items [{:db/id 99 + :block/title "Prop" + :logseq.property/type :number + :db/cardinality :db.cardinality/many}]}} + json-result (format/format-result base-result {:output-format :json}) + edn-result (format/format-result base-result {:output-format :edn}) + parsed-json (js->clj (js/JSON.parse json-result) :keywordize-keys true) + parsed-edn (reader/read-string edn-result)] + (is (= "many" (get-in parsed-json [:data :items 0 :cardinality]))) + (is (nil? (get-in parsed-json [:data :items 0 :db/cardinality]))) + (is (= :db.cardinality/many (get-in parsed-edn [:data :items 0 :db/cardinality])))))) + (deftest test-human-output-add-upsert-remove (testing "upsert block renders ids in two lines" (let [result (format/format-result {:status :ok