Native app: show merge risk apart from the review's quality score - #342
Open
kentdebruin wants to merge 1 commit into
Open
kentdebruin wants to merge 1 commit into
kentdebruin wants to merge 1 commit into
Conversation
The server's osReview summary now carries risk (low/medium/high), recovery (minutes/hours/days/irreversible) and riskFactors beside the 1-5 confidence, which scores quality only. The Swift model dropped all three and its comment still called confidence "how safe this is to merge". OsReviewSummary gains the three fields with a hand-written decoder: a risk or recovery word this build does not know drops to nil instead of failing the whole sessions list. Both native review surfaces say risk as its own axis, the way the web sidebar does: the workspace agent row puts "high risk" between the score and the state in its own ink (the band stays quality), and the session card's fact strip adds a separate "high risk" fact. High and medium take the existing red and yellow status inks, low goes dim, and a stale reading goes faint with the verdict it belongs to. The review sheet's header repeats the two numbers. A DEBUG fixture (OS1_WORKSPACE_REVIEW_FIXTURE) renders the row at every level for the screenshot harness. Tests cover decoding at each level, absent and unknown words, stale readings, and the wording and inks of both presentations. Co-authored-by: Kent de Bruin <[email protected]>
Contributor
🤖 OS review · approve · quality 5/5 · risk lowSafe to merge. No blocking correctness, compatibility, or security issues found. 🟢 Risk low · recovery in minutes Reviewed |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the review-summary wire and UI changes from 47c0699 to the native Swift client (OS1 / OS1Mac).
Why
The server's
osReviewsummary now carriesrisk(low/medium/high),recovery(minutes/hours/days/irreversible) andriskFactorsbesideconfidence, which scores quality only.Models/Session.swiftdropped all three and still described confidence as merge safety, so the workspace review row and the session card never showed risk.What changed
Session.swift):OsReviewSummarygainsrisk: Risk?,recovery: Recovery?,riskFactors: [String]?with a hand-written decoder. An unknown risk or recovery word drops tonilinstead of failing the whole sessions list.WorkspaceReviewRows.swift): the agent row's second line becomes4/5 · high risk · No findings. The band and the score keep the quality tone; only the risk word takes its own ink (red for high, yellow for medium, dim for low, faint when stale).ReviewRow.detailis aTextso one line can carry two inks and still read as one VoiceOver label. Logic lives in a pureAgentReviewDetailso it is testable. The review sheet header repeats both numbers.SessionRowPreview.swift): a separatehigh riskfact on the strip viaPrPreviewFacts.riskFact, with the same tones; the verdict phrase is unchanged.OS1_WORKSPACE_REVIEW_FIXTURE=1renders the row at every level forscripts/capture-ios.ts; the PR-cards fixture now carries risk values.greenInk/yellowInk/redInk,secondaryLabel/tertiaryLabel), which already hold contrast on both platform canvases in both appearances; no web colour values were copied.Tests
SessionTests: decoding at low/medium/high, recovery and factors, an older server without the fields, an unknown word, and a stale review keeping its risk.SessionRowPreviewTests: risk fact per level, absent risk, stale risk goes faint, risk never enters the verdict phrase.AgentReviewDetailTests(new): line wording and inks per level, absent, stale, no score, and a running pass.Verification
xcodebuild buildforOS1(iOS Simulator) andOS1Mac;xcodebuild testonOS1Macfor the six related suites: 120 tests, 0 failures.bun run checkgreen.Started by Kent de Bruin in this OS session