mirror of
https://github.com/lobehub/lobehub.git
synced 2026-09-20 04:56:13 +08:00
Arvin Xu
8a8b999993
* ✨ feat(expertise): distil delivery standards from the checks a reviewer rejected The reviewer's rejections are the only structured record of what they actually require, and nothing read them: expertise ingestion mined conversations only. This turns a settled acceptance round into expertise the same way a topic completion already does, reusing the whole lesson dedup / hit / snapshot path. Trigger: a new round attaching to the acceptance settles the previous one. The product's reject flow ends at a clipboard copy (`onCopyReview`), so no server-side event marks "I finished reviewing" — but the next round landing proves the previous round's judgement is final. Folding into a draft round is deliberately not a trigger: no new round opened, so nothing settled. - `persistDomainRun` now takes an explicit run descriptor instead of assuming a topic/agent, so both ingestion paths share it. - One observation writes one hit per rejection it cites, so `hitCount` counts violations rather than analysis passes. - The prompt is a separate chain: it must generalize ("this divider isn't needed" → "decorative dividers are not added"), cite its sources, and state when the standard does not apply. Restating one rejection verbatim yields a rule that never fires again, which is the failure this pipeline exists to avoid. - Domains resolve through the project the acceptance belongs to, falling back to the owner's always-on domains; when neither exists, a delivery-standards domain is created, owned by the user and merely mounted on the project so a sibling project can mount the same one later. Distillation runs out of band and never throws into the attach path: losing a lesson is cheaper than losing the reviewer's round. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * 🐛 fix(expertise): read the circled frames, and stop losing rounds to `{}` nulls Replayed one real round (7 rejections, all circled) against the pinned model and found two things unit tests cannot see, because a unit test hands the parser a constructed object while the model hands it whatever it feels like. 1. Every `['string', 'null']` field came back as `{}` under a strict json_schema, so `RejectionAnalysisSchema.parse` threw and the entire round was lost. The nullable fields are plain strings now, with "" as the "none" the service maps back to null. A test walks the schema and fails on any resurrected union. 2. The distillation was text-only, which is close to useless here: 616 of this owner's 876 rejections circle a region on a screenshot, and "this isn't aligned" / "this cyan is too light" cannot be understood from the words. One round's frames are now inlined as data URIs, circled ones first, and each circled region is rendered with its label and normalized position so the prompt can point at the frame it belongs to. Inlined rather than linked because a link makes the provider fetch from our storage, which is unreachable for a local or private bucket. Frames are capped and the overflow is named in the prompt, so a withheld frame is never read as proof that nothing was wrong there. Also makes `limits` mandatory prose rather than an optional null. On the first replay the model left it null on all 7 observations; a standard with no stated exemption gets applied everywhere and becomes noise. Verified on the replayed round: parse clean, all 7 limits filled with real exemptions, and re-feeding the same rejections with those 7 lessons present attaches 7/7 with 0 new lessons — the dedup path holds, so repeats do not fragment the library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): keep a distilled standard to what the reviewer actually said Reviewing the first replay's output against the rejections it came from showed the distillation inventing the two things that matter most. `reasoning` was manufacturing design rationale. "This order is reversed" became "follows common messaging interaction conventions and does not match user visual habits" — plausible prose the reviewer never wrote, which a later compile step would then enforce as if they had. It now carries the reviewer's own words, or says outright that they gave no reason. `limits` was inventing exemptions. "Except on mobile or narrow containers", "except in large rich-text editors" — none of it stated by anyone. An invented exemption silently narrows a standard the reviewer stated without limit, so the field is now filled only from a boundary they drew or a contradicting rejection in the same round, and otherwise says so. Generalization also gets a test it can fail rather than the vague "strip the screen names": could a delivery on a completely unrelated screen violate this? A new `subject` field forces the answer to be written down — a model that cannot say what the concrete thing is an example of has not generalized at all. The prompt also caps the climb at what was actually objected to, so one rejected divider does not become a rule about every visual element. Replayed the same production round (7 rejections, all circled) on v2: reasons are the reviewer's own words on all 7, limits honestly empty on all 7, and the abstraction climbs where it should — "the annotation border on a screenshot" became "highlight and annotation lines drawn over content". Known cost, not yet addressed: climbing a level can drop the direction out of a standard. "Place the add-reaction control after the existing reactions" became "maintain the correct order", which is no longer something a delivery can be judged against. Two of seven regressed this way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): explain why a standard holds, and say who supplied the reason The previous pass cut `reasoning` down to the reviewer's own words to stop the distillation inventing rationale. That over-corrected: quoting the complaint back carries no information, duplicates the evidence stored on the hit, and strips out the one thing that lets a standard reach a screen nobody has built yet — the mechanism. "A pale line disappears against a white background, so the reader cannot tell which region was circled" is what makes the rule portable; "the reviewer said the cyan is too light" is not. So the reason is explanatory again, and the real hazard is handled where it belongs: `reasonSource` records whether the reviewer stated it or the distillation supplied it. The compile step downstream turns a lesson's reason into an enforced criterion, so a reason the model invented must never read as something the reviewer said. That field only works if it is decided by an observable rather than a judgement call. Asked "did the reviewer say why", the model marked 3 of 7 as `reviewer` on rejections that were pure instructions — "put it in one row, annotations left, actions right", "there's an extra line here" — which is exactly the direction of error the field exists to prevent. Asked instead "does their text state a consequence or a cause", all 7 came back correct on replay: only "the cyan is too light, I can't see it" kept `reviewer`, because it names the consequence. Inferred reasons are marked in the lesson body so the distinction survives into the UI, and the prompt now says to answer `inferred` when in doubt: under-claiming costs nothing, over-claiming puts words in the reviewer's mouth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): judge a distilled reason by whether it settles a case nobody described "The mechanism is what lets a rule transfer to a new situation" is not just guidance — it is a test the model can run on its own sentence, so the prompt now states it that way: could someone facing a case this round never described settle it using your reason alone? That test is what separates a mechanism from the objection restated in nicer words. "Harms visual tidiness", "feels inelegant", "adds cognitive load" all fail it, because tidiness and elegance are the judgement under review — they settle nothing. The prompt names those phrasings and asks for the observable underneath: what does a person fail to see, misread, mis-click, or have to do twice. Replayed on the same production round, 5 of 7 reasons moved from verdict to mechanism: "loses the value of an inline screenshot" became "the reader cannot make out the content and must click to enlarge every time"; "misleads the user into thinking it is a network fault" gained the action that follows, "and retries blindly". Two did not, and the reason is worth recording rather than tuning away: a purely aesthetic standard has no functional mechanism underneath it. Pressed for one, the model can only produce a synonym of the verdict ("visual noise" for "untidy"). Those lessons want an honest "this is the owner's taste" more than they want an invented mechanism — a distinction worth making explicit once there is enough volume to see how common they are. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): let a standard admit it is taste rather than fake a mechanism Not every standard has a mechanism underneath it. Some of what a reviewer requires is simply what they prefer, and pressing the distillation for a reason anyway produces a synonym of the objection — "visual noise" for "untidy", "fragments related actions" for "I want it in one row". Those read as objective and would be enforced as if they were, which is worse than an admitted preference. `reasonKind` records which one a standard is, orthogonal to `reasonSource`: one says who supplied the reason, the other says whether the reason rests on something observable. The model's own failure is the detector — if the only reason it can produce is a synonym of the complaint, that is the signal to stop and answer "taste". A taste reason still has to be actionable: "the owner does not accept dividers that were not asked for; regions are separated by spacing and container edges alone" says what to do next without pretending to be physics. It is also load bearing downstream. A taste standard has no objective test, so the compile step must not turn it into a criterion that blocks a delivery on its own — noted where the field is declared, since that step lands later. Replayed on the same production round: 4 mechanism / 3 taste, and the three are exactly the ones that had been fabricating mechanisms. The four keep the reasons they had, one of them improving ("also leaks how the backend handles errors"). Both qualifiers are rendered into the lesson body, so the distinction survives into the UI rather than living only in the analysis. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): restate a standard once the reviewer has rejected it several times Ingestion writes a lesson from the first rejection that produces it and, on every rejection after that, only bumps its counters. So a standard that has fired eight times is still worded for the screen it was born on, and still carries no boundary — both of which are measurable on this owner's history. Replayed against 898 circled rejections: of 44 rejections in the same category as a distilled standard but from other acceptances, the standard reproduced the reviewer's actual objection on 4, while firing on 39% of deliveries they had accepted. A rule written from one case and bounded by nothing does exactly that. It is worth fixing rather than routing around, because the repeats are real: grouping those rejections by the standard behind them puts 62% of the spacing complaints and 40% of the unrequested-decoration ones into groups of 2+, largest 12. Placement reaches only 11%, which is why refusing has to be a first-class answer rather than a failure. The pass reads every instance at once, so the wording can climb to what they share instead of what one of them said, and it reads deliveries the reviewer ACCEPTED alongside them, which is the only input that can justify a limit. A boundary inferred from the instances is invented; one read off a delivery they shipped is theirs. On the divider group this is the difference between "输入框内部 工具栏不应添加多余的分隔线" and a standard that exempts danger-action separators in menus, table headers and sibling list rows, each citing the accepted delivery it was read from — verified against those frames rather than taken on trust. Two design points came out of running it on real rows instead of fixtures: `generalized: false` is an answer, not an error. Instances that share a category do not share a standard: "put the status next to the title" and "move topics above the profile" are both placement and are not one rule. The refusal is recorded as a revision anyway, or the same instances are re-read on every subsequent rejection. Taste survives consolidation. Given six spacing rejections the model answered `taste` and left the boundary unwritten rather than manufacturing a mechanism from six data points — which is the behaviour that keeps a preference from being enforced as if it were objective. Consolidation runs after the write and never inside it: a round of rejections that is safely recorded must not roll back because the rewrite of an unrelated standard failed. No schema change — `revision.kind = 'generalize'` and `prevTitle` were designed for this and had no writer until now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ✨ feat(expertise): make every generalized boundary name the delivery it rests on The consolidation prompt asked the model never to invent an exemption, but nothing checked. Its `limits` was one free-text sentence, and the deliveries it cited ("S2") were per-request labels that meant nothing once the request was gone — so "why is this allowed" had no answer, and an invented exemption looked exactly like a real one. `limits` is now a list, each entry citing the shipped deliveries it was read from. `resolveLimits` maps those labels back to check result ids and drops any entry that resolves to nothing: no refs, a rejected instance's ref, a label that was never listed. What survives is written into the lesson and, with its ids, into `revision.evidence` alongside the instances and the shipped set the pass was offered — refusals included, so "not one standard" can be checked against the same rows. Fixes a second way to get a boundary wrong, in the other direction: a rewrite used to replace the lesson's limits wholesale, so a boundary the reviewer had stated could be silently dropped, widening the standard past what they said. A carried-over limit is now marked as such and kept only if its wording is already in the standard; when nothing survives, the existing limits section stays. Replayed on the divider group from production: two exemptions, citing S2 (the profile "⋯" menu that isolates Delete) and S4 (the documents table), resolved to their check result ids; both frames checked by eye. Spacing wrote none, and the mixed placement group still refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * 🐛 fix(expertise): distil the final round, and stop two rounds racing without a queue Three fixes from review, all on paths the replays never exercised. **The last round of every acceptance was never learned from.** Distillation was scheduled only when a later round attached, on the reasoning that a new round is the first server-side proof the previous one is settled. But a terminal accept or reject settles a round just as surely, and it is the only thing that can settle the last one — no later round ever follows it. `stampDecision` now returns the round it stamped and both terminal paths hand it to distillation. Accepting still settles it: a reviewer can accept overall while individual checks were rejected along the way, and those rejections are the material. Triggering a round twice is harmless — the reflection key makes the second pass a no-op. **Without a queue, two rounds settling together raced.** The queue path buys serialization with `flowControl`; the local path had nothing, so both runs could see no bound domain and create duplicate defaults, or distil against the same stale lesson catalog and fork a lesson that should have attached — the persistence lock only serializes the writes, and would record both. Local runs now chain per reviewer, on the same key the queue uses. A failed distillation does not cancel the rounds queued behind it. **Consolidation may no longer promote taste to mechanism.** Replaying one group twice with an unchanged prompt returned `taste` once and `mechanism` once. The two are not symmetric: `mechanism` is the permissive value — it is what lets a standard compile into a criterion that blocks a delivery on its own. Ingestion decides this at the lesson's birth from the reviewer's own words, so a later pass promoting it means model noise can arm the gate with a mechanism nobody stated. Admitting taste needs no such proof, so downgrades still apply. Also corrects what `expertise_lessons.backtest` claims. Its doc described scoring a standard's precision against past decisions as the pre-compile gate; measuring that on 898 circled rejections showed the labels are too incomplete to gate on — a reviewer circles the worst thing in a delivery, so a standard correctly spotting a defect they did not circle reads as a false alarm. The column keeps its place and its emptiness, with the numbers written down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * 🐛 fix(expertise): keep the reviewer's own limits, and move the revision counter with a system pass Three more review findings, all in the consolidation pass. **The next user edit of a consolidated lesson would have failed.** A system pass numbered its revision from the log's maximum but left `expertise_lessons. currentRevision` behind, while `reviseLesson` numbers from `currentRevision + 1` against a unique `(lesson, revision)`. Two system passes, or one after a user edit, and the reviewer's next rewrite collides. The counter now moves inside the same transaction as the revision — including on a refusal, which records a revision too — and it is derived from the larger of the log and the counter, so a lesson whose counter has already drifted repairs itself. **A rewrite could silently drop a boundary the reviewer drew.** `limits` was the model's complete list, so carrying an existing limit depended on it echoing that limit back: omit one of two and the exemption disappears, widening a standard the reviewer had bounded. Existing limits are now kept verbatim by the service and never re-derived from the answer; the model returns only what it is ADDING. The one exception is ingestion's "边界未由评审者说明" placeholder, which only a reader can tell from a real boundary — so the model answers that as a flag, and the placeholder is dropped only when a validated exemption arrives to replace it. **Serialization was keyed too narrowly.** Domains and their bindings are workspace-wide, so two members reviewing at once race exactly as one member's two rounds do, and a per-user key lets each mint a different default domain and split the catalog. Both the local chain and the queue's `flowControl` now key on the catalog: the workspace when there is one, the user otherwise. Replayed against production rows again, since the limits contract changed: the divider group wrote two exemptions citing the profile "⋯" menu and the documents table, both resolving to real check result ids; a variant carrying a pre-existing reviewer limit kept that limit and appended to it. One of those replays came back degenerate — the model emitted collapsed text — and the pass contained it: the invented refs resolved to nothing, so no boundary was written and the reviewer's limit survived untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * 🐛 fix(expertise): sample only the accepted rounds the caller may read **A teammate's creator-only round could end up quoted in a shared standard.** The contrast set was every accepted check in the workspace, but a workspace shares its lesson catalog, not every round in it: a round is creator-only unless it is public. This pass puts a delivery's title, the reviewer's comment and their circled notes into a prompt, then persists the result as a limit the whole workspace reads — so the sample is now restricted the way a reader would be. The caller's own checks, plus other members' only where the round they belong to is public; a check with no round is the caller's own by the scope predicate. **One rejection could count as two.** A repeated label in the model's answer ("R1", "R1") wrote two hits for the same rejection and incremented `hitCount` twice. That count ranks the standards list and decides core-versus-niche, so the distortion is durable even though consolidation later deduplicates the ids. Deduplicated at the write itself rather than at one caller, since that is the only place hits are made and the topic path shares it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>