Repository navigation
feat(denkraum): split ideas and question import onto the store (DR-04c-2) - #674
Conversation
…c-2) Move putIdea and putQuestion from the frozen P1 DeskStore (lines 348-382 and 392-415) into tools/denkraum/store/ledger.mjs as LedgerStore extends ProgressStore. The only change is this.#change( to this.change(; neither operation uses the root agent id. tools/denkraum/store.mjs re-exports the P1 surface (DeskError, latestAnswer, projectQuestionPriority, compareQuestions, migrateState and DeskStore = LedgerStore), so the server and CLI in DR-06 can import one store with every operation. The root agent id stays injected (D2). Test-First: tools/denkraum/store/ledger.test.mjs::an idea revision starts incoming progress and a pending outbox entry and keeps its metadata Test-First: tools/denkraum/store/ledger.test.mjs::store.mjs exposes the P1 surface with every operation on one store Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brings dc34cae (V2 pending, inbox and delivery require the root id even on an empty ledger) up the DR-04/DR-06 stack. Merge, no rebase, as ordered by Root; no conflict, no code change of this package. No-Test: merge commit only; the fix is red-first tested in dc34cae (delivery.test.mjs R669-O1) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brings 5d31ef5 (progress and patch replays check the root id first) up the DR-04/DR-06 stack. Merge, no rebase, as ordered by Root; no conflict, no code change of this package. No-Test: merge commit only; the fix is red-first tested in 5d31ef5 (progress.test.mjs R671-O1) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cuarroc
added
the
review-ok
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39bc8390f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ensure(old || input.expectedRevision === undefined, 'Frage fehlt für diese Revision. Erst neu laden.', 409); | ||
| ensure(!old || input.expectedRevision === old.revision, 'Frage wurde inzwischen geändert. Erst neu laden.', 409); | ||
| if (old?.recommendationDetail && q.sourceRef && !sameRef(old.sourceRef, q.sourceRef) && q.recommendationDetail === undefined) q.recommendationDetail = undefined; | ||
| const next = { ...old, ...q, revision: (old?.revision ?? 0) + 1, createdAt: old?.createdAt ?? now(), updatedAt: now() }; |
There was a problem hiding this comment.
When the system clock advances between these consecutive now() calls, a newly created question receives different createdAt and updatedAt values, while the new regression test explicitly requires them to be equal (ledger.test.mjs:55). This makes the test timing-dependent and can represent a brand-new record as already updated; capture one timestamp and use it for both fields on creation.
Useful? React with 👍 / 👎.
| } | ||
| ensure(old || input.expectedRevision === undefined, 'Frage fehlt für diese Revision. Erst neu laden.', 409); | ||
| ensure(!old || input.expectedRevision === old.revision, 'Frage wurde inzwischen geändert. Erst neu laden.', 409); | ||
| if (old?.recommendationDetail && q.sourceRef && !sameRef(old.sourceRef, q.sourceRef) && q.recommendationDetail === undefined) q.recommendationDetail = undefined; |
There was a problem hiding this comment.
When an imported update changes sourceRef on a question that already has recommendationDetail and omits replacement details, this assignment creates an own recommendationDetail: undefined property in next. The initial putQuestion result therefore has that property, but JSON persistence drops it and a requestId replay returns an object without it, so direct-store callers receive a structurally different replay result; remove the property rather than assigning undefined, or normalize the result before returning it.
Useful? React with 👍 / 👎.
| const index = state.questions.findIndex(x => x.id === q.id); const old = state.questions[index]; | ||
| requireCurrentSource(state, { ...old, ...q }); | ||
| const detail = q.priorityDetail ?? old?.priorityDetail; | ||
| if (detail?.sourceRefs.length) { |
There was a problem hiding this comment.
When an existing question has a legacy priorityDetail without sourceRefs, which is accepted because validateQuestion defaults omitted reference lists to [] (model.mjs:157-170), an update that omits replacement priority details selects that raw stored object and this .length access throws an untyped TypeError. Consequently, an otherwise valid legacy ledger cannot update the question and an HTTP caller would receive an internal error instead of a DeskError; normalize the inherited detail or treat a missing sourceRefs as an empty list.
Useful? React with 👍 / 👎.
Merge Queue Status
This pull request spent 14 minutes 14 seconds in the queue, including 13 minutes 56 seconds running CI. Required conditions to merge
|
Der Denkraum kann jetzt auf dem neuen Speicherkern auch Ideen (mit Kategorie und Priorität) und importierte Fragen speichern, genauso wie im bisherigen Stand P1.
Damit sind alle Speicher-Operationen aus P1 übernommen und über eine einzige Datei
store.mjserreichbar, auf der Server und Befehlszeile (DR-06) aufbauen.Server, Befehlszeile und Oberfläche folgen in eigenen Paketen; zwei Fremdprüfungen und Roots Abnahme stehen noch aus.
Report
Package: DR-04c-2 (Z1), parent ticket
55ea21ce. It is the second half of the DR-04c split announced in #671, under Root decision 14:00Z (Option A, msg-0muzlobgi). Owner: DR-03/04 session (Claudeclaude-opus-5-5[1m]).claude/dr-04c1-denkraum-progress@e35500cea3fa6570fbe7d9a865c7883d1a4742b0(→ feat(denkraum): split pending, inbox and delivery onto the store (DR-04b) #669 → DR-04a: answers, receipts and acks on the store core + C-3 root check #666 → DR-03: DeskStore core with serialized atomic writes + C-2 stored-shape fix #661).What changed (3 new files, 152 ALL)
tools/denkraum/store/ledger.mjs(68):class LedgerStore extends ProgressStorewithputIdeaandputQuestion, moved from P1592b309dlines 348–382 and 392–415.diffof those P1 lines againstledger.mjslines 9–67 shows exactly 2 hunks, boththis.#change(→this.change(.migratein between (383–391) is already the DR-03 core'smigrate.tools/denkraum/store.mjs(2): re-export of the P1 public surface. Fromstore/model.mjs:DeskError,latestAnswer,projectQuestionPriority,compareQuestions,migrateState.DeskStore=LedgerStore. This is the proof's 2-linestore.mjswith the class name of this chain. The root id stays injected with no default (D2), so P1 callers that relied on the hardcoded constant must passrootAgentId.tools/denkraum/store/ledger.test.mjs(82): 5 tests, synthetic ids only.contentRevision. Category is trimmed. Priority is kept. Unchanged metadata carries over to revision 2 and can be reset in revision 3.expectedRevisionwithout id 400; empty title 400. Nothing is written. V1 ledger 409.expectedRevision(missing or stale 409; for a new id 409);createdAtis kept. An import withrequestIdreplays its result, and conflicting reuse gives 409 with no write. An invalid question gives 400.store.mjsexport names equal the P1 export names,DeskStore === LedgerStore, and every P1 operation is on that class.Evidence
e35500cnode --test --test-name-pattern='an idea revision starts incoming progress|store.mjs exposes the P1 surface' tools/denkraum/store/ledger.test.mjsERR_MODULE_NOT_FOUNDforledger.mjs(new module); log sha256e83f5f50bc1f8217…node --test tools/denkraum/store/ledger.test.mjs;npm run test:denkraum(Windows)store,progress,patches,priority,question-import,free-answer,sender,api,servertests) + P1ideas.test.mjs+ this chain'smodel/core/answers/delivery/progress/ledger. The shimstore.mjsis now 4 lines:DeskStore extends LedgerStore, only injecting the root id, no P1 code left. Run:node --test --test-concurrency=1 *.test.mjsCARGO_TARGET_DIR=~/targets/slot5,CARGO_BUILD_JOBS=2, foregroundssh_exec83ca797precommit exit 0 (14:07:56–14:08:10Z)bash scripts/ci/gates.sh lane prepushon83ca797gruen, rust-suite nextest 1873 run / 1873 passed / 7 skipped; log sha256784e723272e468af…bash scripts/ci/red-first.sh7e3a7af, green at the headgit push+git ls-remotePA_PREPUSH=light git push -uafter the full prepush on the same head, exit 0;git ls-remote14:11:03Z =83ca797c0002d820d0ad704365cc455f9701e0b7; #671 branch unchanged ate35500cgit merge --no-ffof d45d610 (#671), no rebase (Root order)d17c1a6brings onlydelivery.mjs/delivery.test.mjs(+18/−3), no conflict, no change of this package's files; trailerNo-Test(fix is red-first tested indc34cae). prepush exit 0 14:33:27–14:35:01Z, nextest 1873 / 1873 passed / 7 skipped, log sha256592e59e50b862cad…; red-first exit 0;git ls-remote14:36:18Z =d17c1a6c08e307dbbbcd4d9fc8d273f56cb7190c. D1 composition rerun with the fixeddelivery.mjs: 101/101git merge --no-ffof5d31ef5(#671), no rebase39bc839brings onlyprogress.mjs/progress.test.mjs(+16/−3), no conflict, trailerNo-Test. prepush exit 0 14:48:37–14:50:06Z, nextest 1873 / 1873 passed / 7 skipped, log sha256bc00ae050790cb24…; red-first exit 0;git ls-remote14:51:09Z =39bc8390f87e9e5dc0d490bf1e97ac138bdb2fcc. D1 composition with the fixedprogress.mjs: 101/101Limits (unchanged, not solved here)
ponytail:comment inputIdeais copied from P1 unchanged. Request replay scans the whole history, part of the open F-01 bounds question.Review
Tier A. OpenAI R1 on
83ca797: approve, no finding.At head
39bc839:review-drdelta-openai-r2): an independent probe confirmed rootless local saves and replays, no receipts and no notifications; verdict approve.review-dr-google-r1-671-674-676): approve.The inherited R669-O1 remainder goes to follow-up DR-04B-FU (PLAN, merged in #695). Root acceptance: 2026-10-08 19:24Z.
putIdeaorputQuestionanswer with success without a root id (replay or empty path)?NICHT ABGEDECKT
rootAgentIdpassed, because the P1 constant is gone (D2). The 101/101 holds only with the scratch shim.#[cfg(windows)]tests (run_in_pty, npm_shim, ConPTY, DPAPI) and the Windows arms of clippy are not covered. Browser smoke, frontend build and workflow gates run only in lanelinux. External services are not checked by any gate. The Windows verdict comes from the merge queue.🤖 Generated with Claude Code