DRU-622 - Show an in-app gate the question its app wrote - #676
Conversation
The shared InAppReview never read ask.label, so an in-app gate showed its buttons with no question above them. Render the optional label as the review prompt, keeping unlabeled asks supported for lent apps. Co-Authored-By: Claude Opus 4.8 <[email protected]>
![druks-reviewer[bot]](https://avatars.githubusercontent.com/u/271179554?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verification — pass (round 1)
The diff is a minimal, three-file change matching the plan's scope exactly: RunControls.tsx gains one line ({ask.label && <div className="review-prompt">{ask.label}</div>}) placed as the first child of .ins-needs, i.e. before the artifact-loading state, critique block, artifact block, questions, note field, and controls. RunControls.test.tsx adds a focused test (shows the app-declared label with its controls) asserting the label and both custom control labels (keep/skip) render together. styles.css adds one .review-prompt rule for spacing.
AC2's backend wiring was traced end to end: Run.get_ask() in backend/druks/durable/models.py and backend/druks/mcp/gateway/services.py are byte-identical to base — get_ask()'s dict-merge ({"label": fallback, ...} | ask semantics) already means an app-declared label overrides the "Review: <title>"/"Review" fallback, so no backend change was needed, matching the plan. GateControls.tsx passes gate.data.ask straight into InAppReview with no transformation, and InAppReview is the sole render path (including the re-exported version installed apps use), so no other consumer needed touching — matching the plan's stated scope boundaries (SubjectPage untouched).
Acceptance criteria
- AC1 — pass.
InAppReviewrendersask.labelas the first element inside.ins-needs, strictly before the artifact/loading block, critique, questions, note field, and controls. New test asserts"Keep this quote?"and controlskeep/skiprender together. - AC2 — pass. Backend fallback (
models.py/services.py) is unchanged and traced throughGateControlstoInAppReview; the dict-merge guarantees an app-declared label wins while the unlabeled path keeps its existing fallback. Existing backend test (test_get_ask_resolves_the_review_artifact) plus the new frontend test jointly exercise the path, per the plan's own verification method.
Verification profile
| Check | Result |
|---|---|
uv run ruff check backend |
pass (local) |
uv run ruff format --check backend |
pass (local, 452 files clean) |
uv run pytest backend/ |
pass (local, 2151 passed) |
npm --prefix frontend run lint |
pass (On Pull Request Frontend / checks, green on head SHA) |
npm --prefix frontend run build |
pass (On Pull Request Frontend / checks, green on head SHA) |
npm --prefix frontend test |
pass (On Pull Request Frontend / checks, green on head SHA) |
No blocking findings. One observational note: no backend test directly asserts an in-app ask's own label survives get_ask()'s dict merge unmodified — coverage relies on Python dict-literal override semantics plus the new frontend test. Consistent with the plan's explicit backend-untouched scope and its own AC2 verification method, so not a gate failure — noting for awareness only.
Code review
Advisory, clean-room pass over the diff (no access to plan/AC/ticket). Summary: small, clean, additive change. ask.label already existed on InputRequest and was already rendered for the external presentation elsewhere (SubjectPage.tsx, EventsPage.tsx) — this closes the same gap for in_app. The conditional-render pattern matches the file's existing style ({critique && (...)}, {artifact && (...)}). The new test asserts rendered text for input it defines itself, not implementation details or pinned production prose, so it survives refactors. No regressions, no scope creep, no dead branches, no unsafe casts or swallowed exceptions found.
One non-blocking finding (left as an inline comment on frontend/src/styles.css:1638): the new .review-prompt CSS rule duplicates .ins-needs-body's five declarations verbatim instead of composing with it, which the file already does elsewhere for a similar case (review-btn review-btn-${control}). Not required for this PR; flagged for awareness.
| .ins-needs-k { font-family: var(--font-mono); font-size: 10px; letter-spacing: 0.1em; text-transform: uppercase; color: var(--bucket-human); margin-bottom: 6px; display: flex; align-items: center; gap: 7px; } | ||
| .ins-needs-body { font-size: 13px; line-height: 1.55; color: var(--text-mid); max-width: 76ch; text-wrap: pretty; } | ||
|
|
||
| .review-prompt { font-size: 13px; line-height: 1.55; color: var(--text-mid); max-width: 76ch; text-wrap: pretty; margin-bottom: 10px; } |
There was a problem hiding this comment.
Advisory (medium, non-blocking): .review-prompt copies .ins-needs-body's five declarations verbatim and adds only margin-bottom: 10px. Both render body copy inside the same .ins-needs wrapper (the wrapper InAppReview returns, and the one SubjectPage.tsx:189 uses for the external presentation's ask.label). Left as two separate rules, they'll drift independently over time — e.g. a future line-height/contrast tweak to .ins-needs-body won't reach .review-prompt.
Consider reusing the pattern already in this file (RunControls.tsx:218, `review-btn review-btn-${control}`): className="ins-needs-body review-prompt" with .review-prompt trimmed to just margin-bottom: 10px. Not required for this PR — flagging for awareness, happy to hear if there's a reason to keep them separate.
1 participant
What changed
Plan
Goal
Show the question an app supplied for an in-app gate. The binding example is
KeepQuote.wait(input_request={"presentation": "in_app", "label": "Keep this quote?", "controls": ["keep", "skip"]}).Implementation
frontend/src/components/RunControls.tsxso the sharedInAppReviewreads the existing optionalask.labeland renders it as the review prompt before the artifact, questions, note box, and controls. Keep unlabeled requests supported becauseInputRequest.labelis optional and the component is also lent to installed apps.frontend/src/components/RunControls.test.tsxproving a declared in-app label is visible with its controls. Existing unlabeled fixtures continue to cover the optional-label path.backend/druks/durable/models.py:backend/druks/mcp/gateway/services.pyalready returnsawait run.get_ask(), so onceInAppReviewconsumes the label, both an app-declared label and the existing"Review: <artifact title>"/"Review"fallback serve the view. No API or type-shape change is needed.Scope boundaries
SubjectPage; it already delegates in-app gates throughGateControlsto the shared review component.Ruled out
SubjectPage: that would leave otherGateControlsand exportedInAppReviewconsumers without the app's question and duplicate presentation logic outside the shared component.Run.get_ask()fallback: existing unlabeled in-app asks would lose the artifact-derived review context even though the gate endpoint already transports it to the client.Acceptance Criteria
InAppReviewrenders the in-app ask'slabelbefore its artifact, questions, note field, and controls; an ask declaring"Keep this quote?"therefore displays that question alongsidekeep/skip.frontend/src/components/RunControls.tsxand its component test infrontend/src/components/RunControls.test.tsx, which must assert the supplied label and controls are rendered in the review.Run.get_ask()reach the shared in-app review: an app-declared label wins, while an unlabeled ask retains the existing"Review: <artifact title>"or"Review"fallback.backend/druks/mcp/gateway/services.pyfromRun.get_ask()throughGateControlstoInAppReview; the existing backend fallback coverage and the new frontend label coverage jointly exercise the path.🤖 Generated with Claude Code