Skip to content

design: validate durable delivery of human phrase decisions - #841

Merged
WaylandYang merged 9 commits into
deeplethe:devfrom
Maya-Kid:codex/stage2-delivery-prototype
Sep 21, 2026
Merged

WaylandYang merged 9 commits into
deeplethe:devfrom
Maya-Kid:codex/stage2-delivery-prototype

Conversation

@Maya-Kid

@Maya-Kid Maya-Kid commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

The durable-delivery reasoning now lives in ADR 0051, indexed in both tables, with a dated contextual note in 0044. The one-off script README is removed; the real Rust regressions stay.

The unused public try_materialize entry point is removed. Production materialize still calls private materialize_in_tx, and production phrase_bindings::decide calls decide_on. Validation still precedes pool acquisition on the existing decide path. The integration target is now human_phrase_materialization_delivery; normal recomputation and process-exit cases use the existing public materializer.

Busy orchestration lives only in materialize_delivery_tests.rs under cfg(test). It acquires the try-lock, calls the same real private body after success, and uses real Deferred handling on Busy. The test observes production materialize blocked in PostgreSQL by that lock, releases it, and verifies completion. The old-to-new assertion map retains rollback, pool release, same-job deferral, late arrivals, reverse/duplicate processing, actual worker startup, bounded deferral, scoped requeue, cost and all three OS process-kill windows.

Linux validation at 7e49b7fd6d555727f169a77d36ab12367bf97003 against dev 69560db5969fae8def8c4e84b7572d841e1dca57:

  • cargo fmt --all --check, cargo clippy --locked --workspace --all-targets -- -D warnings, cargo test --locked --workspace -- --test-threads=1, and cargo build --locked --workspace passed; database and PDF requirements enabled. Workspace: 959 passed, 0 failed, 5 ignored (existing live HTTPS plus the opt-in entries below).
  • Dedicated idle PostgreSQL 16.15 database: both integration parents passed and actually killed/waited for three children (before commit, after acceptance, after projection before ack). The private busy-lock parent passed separately.
  • Splitting decision/enqueue transactions, treating Busy as done, and requiring a model each caused the intended runtime assertion failure. Restored code passed again.

Opt-in commands are in CONTRIBUTING and the test module headers. No production job kind, HTTP 202/status read, Review UI or multi-instance recovery contract is introduced. Partial-projection process kill and failed ack persistence remain future production acceptance; this does not close #800. The proposed contract remains Draft.

Refs #800

…ion handler

Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

The content here is decision material — it states a policy and asks maintainers to approve it — and the thinking in it is good. My objection is only about where it lives.

docs/decisions/README.md states the purpose of that directory plainly: code records what was built and git records when it changed; neither records why, and that directory does. Putting the reasoning in scripts/prototypes/**/README.md and leaving a five-line pointer in the accepted record splits it. A reader who opens the record in six months finds a stub aimed at a directory whose name says "prototype" — with no number, no row in either index table, no status line for an implementing PR to update, and none of the revision discipline the conventions require. The "Decision requested" section is exactly the thing that is supposed to be numbered and indexed.

Please give this its own numbered record in docs/decisions/, and turn the addition to the accepted record into a dated revision note that names it, per Revisions stay in place.

#832 in this same batch is the pattern to copy: it moved its reasoning into docs/decisions/0048-provenance-references-stay-inside-the-knowledge-base.md, added the row to both index tables, opened an issue for the open design question, and left the migration carrying only what SQL has to say.

One thing that is not my call and that I have flagged for the maintainer: whether the executable prototype itself belongs in the repository. scripts/bench/ is tracked, so there is precedent for committed harnesses, but these add a Python harness and a requirements.txt, and that is a policy question rather than a review point.

Separately, and this one does block: try_materialize is public, unused, and in a production crate. Your own addendum says unused prototype helpers alone do not resolve #800 — which is exactly the problem. A pub async fn in utopia-store reads as wired to the next person who greps for it, and dead_code will not warn on it because it is public. Either land it together with the caller that uses it, or keep it out until that caller exists.

The materialize_in_tx extraction and phrase_bindings::decide_on are a different matter and are fine — those are enabling refactors with real callers, and pulling the body out of materialize so a transaction can be owned by the caller is a good change on its own.

@WaylandYang

Copy link
Copy Markdown
Contributor

Decision on the committed prototype directories, so all three of these can move at once: the conclusions belong in a numbered record; the harness stays out of the repository.

The test we are applying is not "prototype versus production". It is: after the decision is made, will anyone run this again?

  • If yes, it is a bench and it belongs next to what it measures. scripts/bench/govern.mjs is tracked for exactly that reason — 0025 cites it, and when the governance prompt changes someone reruns it against the labelled pair set. Its value recurs.
  • If no, the finding goes in the record and the scaffolding does not enter git. That was the call on the HotpotQA harness earlier: the durable artifact was the number, not the rig.

There is a second reason, and it is the stronger one: an executable specification that is not the implementation is a second source of truth. This is the thing the codebase avoids everywhere else. 0032 refuses to store an expression as a string and evaluate it at run time; the rule model is structured precisely so that what the page shows is generated from what runs and cannot drift from it. A policy module that encodes the rules beside the validator rather than inside it is that same drift, with the added property that nothing will ever fail when the two disagree.

So: please lift the reasoning and the decisions requested into a numbered record under docs/decisions/, add the row to both index tables, and turn the paragraph added to the accepted record into a dated revision note naming it, per Revisions stay in place. #832 is the pattern — it moved its reasoning into docs/decisions/0048-provenance-references-stay-inside-the-knowledge-base.md, indexed it, and opened an issue for the question it wanted maintainers to settle.

None of this is a judgement on the work itself. The analysis is good and the experiments clearly did their job; the argument is only about what survives them.

Applied to this PR, with one carve-out:

  • scripts/prototypes/phrase-delivery/README.md — the decisions requested go in the record.
  • crates/utopia-store/tests/phrase_delivery_prototype.rs should stay. It is not a prototype: it is a Rust test in the real crate, exercising the real schema through the real store functions, and it records genuine database, process-exit and mutation evidence. Please just rename it to say what it tests rather than that it is a prototype — the word is what makes it look disposable.
  • try_materialize still needs an answer, separately from all of this: it is public, unused, and dead_code will not warn on it.

…prototype

Signed-off-by: dada-yan <BinjunYann@gmail.com>
…izer.

Signed-off-by: dada-yan <BinjunYann@gmail.com>
@WaylandYang

Copy link
Copy Markdown
Contributor

This is still marked as a draft, so I have not merged it — a draft is your signal that it is not finished, and flipping that is your call rather than mine.

From my side the rework answers everything I raised: the reasoning is now a numbered record with rows in both index tables, the accepted record carries a dated revision note pointing at it, and the committed harness is gone. CI is green on all four checks and it merges cleanly onto dev.

Mark it ready for review when you consider it done and I will merge it.

@Maya-Kid
Maya-Kid marked this pull request as ready for review September 21, 2026 03:58
Maya-Kid and others added 2 commits September 21, 2026 14:35
Signed-off-by: dada-yan <BinjunYann@gmail.com>
# Conflicts:
#	docs/decisions/README.md
@WaylandYang
WaylandYang merged commit 474b904 into deeplethe:dev Sep 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A phrase decision recomputes the typed graph inside the request

2 participants