Skip to content

Add a PasteValue DoKind so reflection answers need substance (fixes #81) - #95

Merged
Jolah1 merged 2 commits into
Jolah1:mainfrom
Nitjsefnie-OSC:feat/81-pastevalue-dokind
Jul 21, 2026
Merged

Add a PasteValue DoKind so reflection answers need substance (fixes #81)#95
Jolah1 merged 2 commits into
Jolah1:mainfrom
Nitjsefnie-OSC:feat/81-pastevalue-dokind

Conversation

@Nitjsefnie

Copy link
Copy Markdown
Contributor

Implements #81: missions 102/103 get their own DoKind::PasteValue instead of riding the Knowledge arm, and verify_proof enforces a minimum-length floor on the reflection — nothing more (no quality judgment, per the issue's explicit scope).

  • mission.rs: 100 | 101 | 104 => Knowledge, 102 | 103 => PasteValue (the compiler then flagged exactly one non-exhaustive match — verify_proof — which is the whole touch-set).
  • missions.rs: MIN_PASTE_VALUE_LEN = 20, counted in chars() (UTF-8-safe) on the trimmed proof; failure message is written as an invitation ("say a little more — …"), not a scolding.
  • Tests: a unit test pinning 102/103 out of the Knowledge arms, and an API test proving a short filler is rejected (400) while a real sentence passes — the latter fails on unpatched source with left: 200, right: 400, so it genuinely bites. One existing test needed its filler text lengthened for 102/103 (its 12-char "acknowledged" placeholder now trips the floor — which is exactly the behavior the issue asks for).

The floor value is the one judgment call the issue leaves open — every other length check in verify_proof is protocol-derived (sha256=64, npub 32-90), so there was no precedent to borrow. 20 chars rules out one-character/one-word answers while still admitting a bare docs link; it's a constant, so trivially yours to tune (or switch to a word count) if you'd prefer a different bar.

cargo build --release --locked and cargo test green (31 lib + 31 integration); no frontend change needed (paste-value inputs and per-mission maxLength already exist there); no migration (completions record generically, same as Knowledge).


Generated by Claude Fable 5 (brief, review), Claude Sonnet 4.6 (implementation)

@vercel

vercel Bot commented Jul 18, 2026

Copy link
Copy Markdown

@Nitjsefnie is attempting to deploy a commit to the jolah1's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Jolah1

Jolah1 commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Implements #81: missions 102/103 get their own DoKind::PasteValue instead of riding the Knowledge arm, and verify_proof enforces a minimum-length floor on the reflection — nothing more (no quality judgment, per the issue's explicit scope).

  • mission.rs: 100 | 101 | 104 => Knowledge, 102 | 103 => PasteValue (the compiler then flagged exactly one non-exhaustive match — verify_proof — which is the whole touch-set).
  • missions.rs: MIN_PASTE_VALUE_LEN = 20, counted in chars() (UTF-8-safe) on the trimmed proof; failure message is written as an invitation ("say a little more — …"), not a scolding.
  • Tests: a unit test pinning 102/103 out of the Knowledge arms, and an API test proving a short filler is rejected (400) while a real sentence passes — the latter fails on unpatched source with left: 200, right: 400, so it genuinely bites. One existing test needed its filler text lengthened for 102/103 (its 12-char "acknowledged" placeholder now trips the floor — which is exactly the behavior the issue asks for).

The floor value is the one judgment call the issue leaves open — every other length check in verify_proof is protocol-derived (sha256=64, npub 32-90), so there was no precedent to borrow. 20 chars rules out one-character/one-word answers while still admitting a bare docs link; it's a constant, so trivially yours to tune (or switch to a word count) if you'd prefer a different bar.

cargo build --release --locked and cargo test green (31 lib + 31 integration); no frontend change needed (paste-value inputs and per-mission maxLength already exist there); no migration (completions record generically, same as Knowledge).

Generated by Claude Fable 5 (brief, review), Claude Sonnet 4.6 (implementation)

Thanks for taking, Nice work!
This is clean and well-scoped.

Two things before merge:

  1. Needs a rebase
  2. Non-blocking, up to you: a one-line note on the PasteValue variant clarifying it covers 102/103 only. mission 106 also renders a paste-value input on the frontend but correctly stays Knowledge (it's a numeric naira-quote
    input, not a reflection). Just so the frontend kind → backend DoKind divergence doesn't trip up the next reader.

Missions 102/103 render a paste-value input on the frontend, but
server-side mapped to DoKind::Knowledge, so any non-empty string
cleared them. Split them into a new DoKind::PasteValue that enforces
a minimum length (MIN_PASTE_VALUE_LEN = 20 chars, a judgment call
documented at the constant), so a one-character reflection can't
undercut the exercise while a short-but-real answer (a bare docs
link, a one-line restatement) still passes. Quality of the reflection
is still not judged, and the rejection message reads as an invitation
to say more rather than a scolding.

Adding the variant was a compile error in verify_proof() until
handled, as the issue predicted; that's the only other call site.

Closes Jolah1#81

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Nitjsefnie
Nitjsefnie force-pushed the feat/81-pastevalue-dokind branch from f4664ca to 51f5166 Compare July 21, 2026 01:08
@Nitjsefnie

Copy link
Copy Markdown
Contributor Author

Thanks! Rebased onto main. The only conflict was the do_kind match arm — main's new 106..=110 => Knowledge line and this PR's 102/103 split land on independent mission tokens, so they compose cleanly:

  • 100 | 101 | 104 => Knowledge
  • 102 | 103 => PasteValue
  • 106 | 107 | 108 | 109 | 110 => Knowledge (kept exactly as main added it)

And added the note you suggested on the PasteValue variant — it calls out that 106 renders a paste-value input on the frontend but intentionally stays Knowledge (numeric naira-quote, not a reflection), so the length floor doesn't apply there. Good catch; that divergence would absolutely trip up the next reader.

cargo build --release --locked and cargo test are green on the current base (32 lib + 34 integration). Ready when you are.

@Nitjsefnie

Copy link
Copy Markdown
Contributor Author

Quick note on the red CI: it's the e2e (smoke) job's badge celebration / share modal on tree completion test timing out around the share modal — and that same job is red on main right now (backend green, e2e red there too), so it looks pre-existing rather than from this change. This PR's backend (cargo) job is green, and the e2e run shows no 4xx on any mission submission, so the 102/103 length floor isn't rejecting anything in the walkthrough. Happy to help chase the share-modal e2e separately if it's useful, but it seems independent of this diff.

@Jolah1 Jolah1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: correct and well-scoped, one copy-rule violation to fix before merge.

Correctness, checks out

  • The floor is applied to the trimmed proof. complete_mission passes body.proof.trim() into verify_proof, so 20 spaces can't clear the bar.
  • proof.chars().count() is the right measure (UTF-8 safe, unlike len()).
  • The frontend already trims and submits paste-value input generically and needs no change, as claimed; the server's 400 message surfaces through the existing error path.
  • No migration risk: completions are recorded generically, and since no mission was added to a path, existing badges are unaffected. Learners who already passed 102/103 with short proofs keep their completions.
  • The API test walks prerequisites 100/101 first, so it exercises the real gate rather than bypassing it.
  • The mission 106 carve-out comment (paste-value UI but numeric quote, stays Knowledge) is a thoughtful guard against someone "fixing" it later.

Must fix: em dash in user-facing copy

The new error message violates the project's standing copy rule (no em/en dashes in user-facing text):

"say a little more — at least {MIN_PASTE_VALUE_LEN} characters, so there's something real to reflect on"

This string is shown to learners in the UI. Use a colon or comma instead, e.g. "say a little more: at least 20 characters, so there's something real to reflect on". The test only asserts err.contains("more"), so the fix won't break it. (Em dashes in code comments are fine; the rule covers user-facing strings.)

Nit

  • In open_source_graduation_demands_a_parseable_github_proof, the comment says "see paste_value_missions_require_more_than_a_word below", but that test sits above it in the file.

Explicitly not flagging

The floor is trivially gameable ("aaaaaaaaaaaaaaaaaaaa" passes), but the issue explicitly scopes out quality judgment, and the PR is honest about that trade-off. The 20-char value is a reasonable call and is a named constant if the maintainers want to tune it.

- verify_proof's paste-value floor message used an em dash in user-facing copy
  (violates the no-em/en-dash rule); use a colon. The test still asserts
  err.contains("more").
- Stale "below" -> "above" in the open_source_graduation test comment (the
  referenced test sits above it).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Nitjsefnie

Copy link
Copy Markdown
Contributor Author

Fixed both:

  • The floor message now uses a colon instead of the em dash: "say a little more: at least 20 characters, so there's something real to reflect on". The test still asserts err.contains("more"), so it's unaffected.
  • belowabove in the open_source_graduation_demands_a_parseable_github_proof comment.

Pushed as b241d38.

One FYI while I was in there (out of this PR's scope, so I left it): backend/src/routes/missions.rs:458 has the same em-dash-in-user-facing-copy — the signet check's AppError::BadRequest("transaction not found on Mutinynet or signet — wait for it to propagate…"). Same rule; happy to send a separate one-line PR to fix it if that's useful.

@Jolah1 Jolah1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both review items are addressed in b241d38, verified against the commit:

  • Floor message now reads "say a little more: at least 20 characters, so there's something real to reflect on", colon instead of em dash. The remaining em dashes in the diff are code comments and a test's simulated learner input, which the copy rule doesn't cover.
  • The stale "below" reference in open_source_graduation_demands_a_parseable_github_proof now says "above".

On CI: confirmed the e2e (smoke) failure is pre-existing, the same job is red on main's latest run (backend and frontend green in both places), so it's independent of this diff. I've approved the pending workflow run for b241d38; backend was green on the previous commit and the delta is two lines of string/comment, so no surprises expected.

On the FYI: confirmed the pre-existing em dash in the signet "transaction not found" message, and yes, a separate one-line PR for it would be welcome.

Approving. Thanks for the quick turnaround and the clean rebase.

@Jolah1
Jolah1 merged commit ce03c6f into Jolah1:main Jul 21, 2026
2 of 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.

2 participants