Skip to content

fix(certification): show demo params and result before discussion in all Sage demo turns#4944

Open
bokelley wants to merge 1 commit into
mainfrom
claude/issue-4940-demo-result-visibility
Open

fix(certification): show demo params and result before discussion in all Sage demo turns#4944
bokelley wants to merge 1 commit into
mainfrom
claude/issue-4940-demo-result-visibility

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #4940

During the A2 "Your first media buy" certification module, Sage was calling get_products (and other demo tools) and then discussing the results without first rendering them to the learner — violating the teaching rule "NEVER reference content you haven't shown." The learner also couldn't see the request parameters (brief text, brand domain) before the call fired. This PR patches all three prompt-string injection sites in certification-tools.ts that emit demo instructions to Sage.

Non-breaking justification: purely modifies string constants injected into Sage's LLM context at runtime; no protocol schema changes, no TypeScript interface changes, no database migrations, no assessment criteria or module IDs changed. Existing credentials are unaffected.

Changes:

  • buildCertificationContext (line 641) — start_certification_module path: replaces "Show, then discuss" with an explicit TWO-STEP SEQUENCE requiring Sage to (1) state request parameters before the tool call and (2) paste the full result verbatim before any interpretive text.
  • createCertificationToolHandlers demo-scenarios block (line 1606) — get_certification_module path: same two-step mandate added after the "Let me show you..." anchor phrase; also adds the explicit escape hatch for the 150-word cap.
  • createCertificationToolHandlers live-demos block (line 1747) — get_certification_module context-reload path: new injection; previously had no sequencing instruction, meaning trimmed-session reloads could reproduce the bug immediately.

Pre-PR review:

  • code-reviewer: approved — no blockers; nits addressed (capitalisation aligned to TWO-STEP SEQUENCE (mandatory): across all three sites, "summarise" → "reference", "full" added to site-1606, changeset body updated to cover all demo tools)
  • education-expert: approved — blocker (missing third injection site at line 1747) addressed; nits noted (150-word/ellipsis tension is consistent across all three sites; disengaged-learner vs. demo-limit latent conflict predates this PR and is out of scope)

Note on build errors: The repo has 4892 pre-existing TypeScript errors on main (missing @types/node environment configuration). My diff introduces zero new errors — confirmed by npx tsc --noEmit 2>&1 | grep certification-tools returning empty.

Triage-managed PR. This bot does not currently iterate on review comments or PR conversation threads (only on the source issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num> → fix → push.
  • Or re-trigger: comment /triage execute on the source issue.

See #3121 for context.

Session: https://claude.ai/code/session_01KNxrd3yCz6idFsLrRJmhBx


Generated by Claude Code

… sites

Issue #4940: Sage was discussing demo tool results without first rendering
them to the learner, violating the teaching rule "NEVER reference content
you haven't shown." Three prompt-string injection sites in
certification-tools.ts now mandate a TWO-STEP SEQUENCE: (1) announce
request parameters in plain language before the tool call; (2) paste the
full result verbatim in the learner-facing message before any interpretive
text. Applies to buildCertificationContext (start path), the start_
certification_module demo block, and the get_certification_module live-demos
reload block.

https://claude.ai/code/session_01KNxrd3yCz6idFsLrRJmhBx
@bokelley bokelley added the claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage. label May 22, 2026
@bokelley bokelley marked this pull request as ready for review May 26, 2026 01:36
Copy link
Copy Markdown
Contributor

@aao-release-bot aao-release-bot Bot left a comment

Choose a reason for hiding this comment

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

LGTM. Follow-ups noted below. Three-site coverage is right — the trimmed-session reload path at L1749 was the load-bearing catch; without it the bug reproduces immediately after context truncation.

Things I checked

  • All three sites carry compatible TWO-STEP SEQUENCE language with matching escape clauses (certification-tools.ts:641, :1606, :1749). The verbatim-or-excerpt-with-"…" wording is identical across sites.
  • The new rule does not contradict the pre-existing "NEVER reference content you haven't shown" at :645; site :1606 explicitly cites it as the rule being enforced for demo turns.
  • Changeset .changeset/fix-a2-demo-result-visibility.md uses the empty ---\n--- frontmatter, which matches the established convention in this repo for non-package-bumping changes (sampled three recent .changeset/fix-*.md files).
  • 150-word cap tension is acknowledged in-prompt at all three sites with "show the result first and discuss it next turn." The two-turn demo pattern is now the documented expectation.
  • No schema, auth, migration, or wire surface touched. Pure runtime prompt content.

Follow-ups (non-blocking — file as issues)

  • Fourth surface drift. TEACHING_METHODOLOGY at certification-tools.ts:156-226 (appended via lines.push(TEACHING_METHODOLOGY) at :1809) still carries the older "describe what the result would look like" framing without the show-params-first sequence. Not a contradiction — TEACHING_METHODOLOGY is general principle, the three patched sites are turn-mechanics — but it's now the weakest link, and a future editor of the canonical methodology constant won't see they need to mirror three other sites. Worth folding the two-step rule into TEACHING_METHODOLOGY as the single source and having :641/:1606/:1749 reference it by name. Three near-duplicate paragraphs is exactly how drift starts.
  • Over-application to capstone and build-project tracks. The injections at :1606 and :1749 fire on get_certification_module regardless of module track. CAPSTONE_METHODOLOGY at :233-245 does not carry the "NEVER reference content you haven't shown" rule because Sage at capstone is assessing, not demonstrating — but a capstone learner running a lab phase now gets a mandatory two-step demo instruction anyway. BUILD_PROJECT_METHODOLOGY already says "present the tool's response exactly as returned" at :110, so the new rule is redundant there. The isBuildProject guard pattern at :1804-1807 is the model to mirror.
  • Disengaged-learner re-engagement exception. The modality at :203 prescribes "run a demo" as the re-engagement move. A disengaged learner getting a turn of "I'm about to query X," then a turn of pasted JSON, then a turn of discussion is more disengaged, not less. One sentence exception: when running a demo to re-engage a checked-out learner, params and result may collapse into one message provided params still appear before result.

Minor nits (non-blocking)

  1. PR body misdescribes site 1747. Body calls it the "context-reload path"; reading the handler, handlers.set('start_certification_module', ...) at :1656 puts this site inside the start-module handler's lesson-plan formatter, not a reload path. Code change at that site is correct — only the description is off.
  2. Redundant escape clause inside site 641. Line :645 already states the show-first-discuss-next-turn rule. Site :641 restating it inside the same emitted prompt is mildly redundant. No functional risk; reinforcement is the likely intent.

Notable choice: PR title bills this as "all three Sage demo turns" while TEACHING_METHODOLOGY quietly remains a fourth surface with the older wording. Worth a follow-up before the next learner-feedback cycle.

Approving on the strength of the three-site coverage plus the explicit changeset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A2 certification: get_products demo result not shown to learner before discussion

2 participants