design: validate a conservative expression declaration policy - #839
Conversation
… rule writes Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
|
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.
Please give this its own numbered record in #832 in this same batch is the pattern to copy: it moved its reasoning into One thing that is not my call and that I have flagged for the maintainer: whether the executable prototype itself belongs in the repository. Specific to this one: the unit policy table and the |
|
Adding one more, on The reason for isolating it is understood and half right — But the isolation costs more than it saves, because of what the page is being asked to answer. 0032's open question is "How deep before the picker loses", with the record's own example being that
So what it demonstrates is the interaction shape in a mock, and the shape was not in doubt. Put it in The policy module and its tests are a different matter; |
|
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?
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 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:
|
…n-policy Signed-off-by: dada-yan <BinjunYann@gmail.com>
…ation decisions. Signed-off-by: dada-yan <BinjunYann@gmail.com>
|
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 Mark it ready for review when you consider it done and I will merge it. |
Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
WaylandYang
left a comment
There was a problem hiding this comment.
Picking this up after the ready-for-review flip — the rework matches what was asked
for on the 20th/21st, and nothing since then changes the shape: two index-sync
commits to keep 0049's rows next to 0050/0051 as they landed.
Verified on a merge of this branch onto current dev (after #833/#845/#860/#864):
merges cleanly, both README index rows land in order between 0048 and 0050,
pnpm guard 99 files compliant, pnpm test 132 passed (the three new
expressionDraft cases are collected by vitest, not just present), pnpm build
including type-check passes. No backend, no migration, both language packs updated.
What lands is a proposed record, same as 0047/0048/0050 — the exact spelling
of 1, the known-unit allowlist and whether ratios are in the first cut stay open
in the record's own status line for whoever settles them. The unlisted route is
read-only, non-persisting, and says so on screen; it stays unlisted until 0049 is
approved and the implementation folds it into RulesPanel, per the record's own
plan.
LGTM.
ADR 0032 asks for declaration checks and a usable nested picker, but the standalone model did not exercise the real interface or a real knowledge base. This revision puts the decision in ADR 0049 and replaces the disposable picker with a reusable controlled React editor at the unlisted
/kb/$kbId/expression-draftroute.The route uses the existing authenticated ontology/rules APIs, SearchSelect/Dropdown/Input controls, design tokens and both language packs. It only previews an unsaved structured draft. It does not enable unit validation, save rules, change B1 metadata editing or change dependency analysis. Unsupported expressions stay unconverted; incomplete constants and missing references cannot become a zero or a default attribute.
The policy module, Node model tests, PostgreSQL probe and standalone HTML were archived outside the repository and removed from the PR tree. ADR 0049 preserves their historical findings, exact source revision and limitations. Both index tables are updated; 0032 has a dated note beside its unit-check discussion.
Validation at
64206696689f1901f5cfd3420553a26d70b8de3aon latest dev69560db5969fae8def8c4e84b7572d841e1dca57:pnpm guard,pnpm typecheck,pnpm test(119 passed), andpnpm buildpassed on Linux, Node 22.23.2 / pnpm 10.2.1.Four levels remain a long vertical form on a narrow screen; these checks are not a human usability score. Exact
1, the known-unit allowlist and first-cut ratio policy remain pending. There is no B2 persistence flow or production unit validator. Draft status is intentional.Refs #488