diff --git a/e2e/helpers/selectors.ts b/e2e/helpers/selectors.ts index caa1dcb5..e9cf0033 100644 --- a/e2e/helpers/selectors.ts +++ b/e2e/helpers/selectors.ts @@ -211,7 +211,7 @@ export const feedback = { scoreBadge: '.attempt-score-badge', /** Attempt selector dropdown trigger */ attemptSelector: '.attempt-dropdown-trigger', - /** Metric cards (correctness and abstraction) */ + /** Metric cards: correctness, and abstraction (AbstractionCard.vue, also below the probeable_spec ladder) */ metricCard: '.metric-card', /** Correctness progress bar */ correctnessBar: '.progress-bar-fill', @@ -283,11 +283,13 @@ export const probe = { // --------------------------------------------------------------------------- // Code submission feedback (activities/feedback/CodeSubmissionFeedback.vue) +// Both probeable types use it via ProbeableFeedback.vue (ladder layout); +// probeable_spec quotes the explanation (.explanation-text) instead of the code. // --------------------------------------------------------------------------- export const codeFeedback = { /** Ordered test ladder (pips + ledger); rendered once grading finishes */ ladder: '.feedback-container .test-ladder', - /** One pip per test, in order: .pass (button) / .fail / .lock */ + /** One pip per test, in order: .pass / .fail / .lock */ pip: '.test-ladder .pip', pipPass: '.test-ladder .pip.pass', pipFail: '.test-ladder .pip.fail', @@ -304,6 +306,6 @@ export const codeFeedback = { attemptPanel: '.attempt-dropdown-panel[role="menu"]', /** One attempt in the dropdown (.attempt-tests 'passed/total') */ attemptItem: '.attempt-item-minimal', - /** Debug button on the failing row */ + /** Debug button on the failing row (not rendered for probeable_spec) */ debugButton: '.debug-btn', } as const; diff --git a/e2e/student/probeable-spec.spec.ts b/e2e/student/probeable-spec.spec.ts index 3d3f3530..a0b04822 100644 --- a/e2e/student/probeable-spec.spec.ts +++ b/e2e/student/probeable-spec.spec.ts @@ -8,9 +8,14 @@ * pass is graded 'complete' (ProbeableSpecHandler: segmentation disabled -> * complete) with no abstraction step. * - * Grading in the E2E stack: CELERY_EAGER runs the EiPL pipeline inside - * POST /api/submit/, and USE_MOCK_OPENAI returns the reference solution as - * all 5 LLM variations, so every variation passes both tests. + * Grading in the E2E stack: CELERY_EAGER runs the spec pipeline inside + * POST /api/submit/. The explanation generates ONE code version, and + * USE_MOCK_OPENAI returns the reference solution for it, so it passes both + * tests. + * + * Feedback is the same ordered test ladder as probeable_code (see + * probeable-feedback.spec.ts): the top panel quotes the explanation instead of + * the code, and there is no debug button. * * Fresh users have no AI processing consent. probeable_spec submissions are * not consent-gated yet (#161); submit tests grant consent through the API up @@ -22,7 +27,7 @@ import { test, expect, Page } from '@playwright/test'; import { setAceEditorValue } from '../helpers/ace-editor'; -import { probe, feedback, problemSet } from '../helpers/selectors'; +import { codeFeedback, probe, problemSet } from '../helpers/selectors'; import { fillProbeArgs } from '../helpers/probe'; import { FreshUser, @@ -78,6 +83,30 @@ async function probeX(page: Page, value: string): Promise { expect(response.status()).toBe(200); } +/** Attempts badge: 'n/total' and the score, preceded by its status icon. */ +async function expectAttemptBadge(page: Page, attemptText: string, score: string): Promise { + const trigger = page.locator(codeFeedback.attemptTrigger); + await expect(trigger.locator('.attempt-text')).toHaveText(attemptText, { timeout: GRADE_TIMEOUT }); + await expect(trigger.locator('.attempt-score-badge')).toHaveText(new RegExp(`^\\S\\s*${score}%\\s*$`)); +} + +/** Both tests pass: two green pips, two passed rows, nothing failing or locked. */ +async function expectAllPassLadder(page: Page): Promise { + await expect(page.locator(codeFeedback.pip)).toHaveCount(2); + await expect(page.locator(codeFeedback.pipPass)).toHaveCount(2); + await expect(page.locator(codeFeedback.pipFail)).toHaveCount(0); + await expect(page.locator(codeFeedback.pipLock)).toHaveCount(0); + await expect(page.locator(codeFeedback.lockedRow)).toHaveCount(0); + await expect(page.locator(codeFeedback.failingItem)).toHaveCount(0); + + const rows = page.locator(codeFeedback.passingItem); + await expect(rows).toHaveCount(2); + await expect(rows.nth(0).locator('.test-call')).toHaveText('square(3)'); + await expect(rows.nth(0).locator('.result')).toHaveText('9'); + await expect(rows.nth(1).locator('.test-call')).toHaveText('square(-2)'); + await expect(rows.nth(1).locator('.result')).toHaveText('4'); +} + /** Put the explanation in #promptEditor, submit, and require a 2xx. */ async function submitExplanation(page: Page, text: string): Promise { await setAceEditorValue(page, problemSet.promptEditor, text); @@ -137,51 +166,36 @@ test.describe('C24 Probeable Spec', () => { await expect(page.locator(probe.historyModal)).toHaveCount(0); }); - test('C24b explanation submit grades 100% with both tests shown', async ({ page }) => { - // One mock-LLM grading: 5 variations x 2 tests in the Docker sandbox. + test('C24b explanation submit grades 100% on the test ladder', async ({ page }) => { + // One mock-LLM grading: 1 generated version x 2 tests in the Docker sandbox. test.setTimeout(GRADE_TIMEOUT + 30_000); expect(EXPLANATION.length).toBe(60); await openSpecProblem(page, 'c24b', { aiConsent: true }); await submitExplanation(page, EXPLANATION); - // All 5 mock variations are the reference solution and segmentation is + // The generated version is the reference solution and segmentation is // disabled for this problem, so the grade is complete at 100%. - await expect(page.locator('.notification-container .notification.success')).toContainText( - 'Problem 6: Variations: 5/5 passing', - { timeout: GRADE_TIMEOUT }, - ); + await expect( + page.locator('.notification-container .notification.success .notification-message'), + ).toHaveText('Problem 6: 2/2 tests passed', { timeout: GRADE_TIMEOUT }); + + await expect(page.locator(codeFeedback.ladder)).toBeVisible({ timeout: GRADE_TIMEOUT }); + await expectAttemptBadge(page, '1/1', '100'); - const container = page.locator(feedback.container); - await expect(container.locator(feedback.header)).toBeVisible({ timeout: GRADE_TIMEOUT }); - await expect(container.locator('.attempt-text')).toHaveText('1/1'); - await expect(container.locator(feedback.scoreBadge)).toHaveText('100%'); + // Top panel quotes the explanation; the generated code is not shown. + const container = page.locator('.feedback-container'); await expect(container.locator('.explanation-text')).toHaveText(EXPLANATION); + await expect(container.locator('.code-section')).toHaveCount(0); - const correctnessCard = container.locator(feedback.metricCard).first(); - await expect(correctnessCard.locator('.status-metric')).toHaveText('5/5 pass'); - await expect(correctnessCard.locator('.status-label')).toHaveText('Clear'); - - // Test rows live in the correctness modal. probeable_spec is outside the - // #165 truncation rule, so every variation lists both tests, the hidden - // one included. - await correctnessCard.click(); - const modal = page.locator('.modal-content.correctness-modal'); - await expect(modal).toBeVisible(); - await expect(modal.locator('.badge-level')).toHaveText('5/5 Versions Pass'); - await expect(modal.locator('.version-pill')).toHaveCount(5); - - for (let i = 0; i < 5; i++) { - await modal.locator(`#version-tab-${i}`).click(); - const panel = modal.locator(`#version-panel-${i}`); - await expect(panel).toBeVisible(); - await expect(panel.locator('.details-tests .section-meta')).toHaveText('2/2'); - const tests = panel.locator('.tests-list .test-item'); - await expect(tests).toHaveCount(2); - await expect(panel.locator('.tests-list .test-item.passing')).toHaveCount(2); - await expect(tests.nth(0).locator('.test-call')).toHaveText('square(3)'); - await expect(tests.nth(1).locator('.test-call')).toHaveText('square(-2)'); - } + // Every test passes, so nothing is locked: both rows, the hidden one + // included, appear as passed one-liners. + await expectAllPassLadder(page); + + // The student did not write the code: no debugger. + await expect(container.locator(codeFeedback.debugButton)).toHaveCount(0); + // Segmentation is disabled: no abstraction card. + await expect(container.locator('.metric-card')).toHaveCount(0); }); test('C24c description length bounds gate the submit button', async ({ page }) => { @@ -208,22 +222,21 @@ test.describe('C24 Probeable Spec', () => { }); test('C24d resubmitting shows 2 attempts', async ({ page }) => { - // Two mock-LLM gradings, each 5 variations x 2 tests in the Docker sandbox. + // Two mock-LLM gradings, each 1 generated version x 2 tests in the Docker sandbox. test.setTimeout(2 * GRADE_TIMEOUT + 30_000); const user = await openSpecProblem(page, 'c24d', { aiConsent: true }); - const container = page.locator(feedback.container); await submitExplanation(page, EXPLANATION); - await expect(container.locator('.attempt-text')).toHaveText('1/1', { timeout: GRADE_TIMEOUT }); + await expectAttemptBadge(page, '1/1', '100'); await submitExplanation(page, 'It multiplies the integer argument by itself and returns it.'); - await expect(container.locator('.attempt-text')).toHaveText('2/2', { timeout: GRADE_TIMEOUT }); - await expect(container.locator(feedback.scoreBadge)).toHaveText('100%'); + await expectAttemptBadge(page, '2/2', '100'); + await expectAllPassLadder(page); - await container.locator(feedback.attemptSelector).click(); - const panel = container.locator('.attempt-dropdown-panel[role="menu"]'); + await page.locator(codeFeedback.attemptTrigger).click(); + const panel = page.locator(codeFeedback.attemptPanel); await expect(panel).toBeVisible(); - const items = panel.locator('.attempt-item-minimal'); + const items = panel.locator(codeFeedback.attemptItem); await expect(items).toHaveCount(2); await expect(items.locator('.attempt-num')).toHaveText(['2', '1']); await expect(items.locator('.attempt-score-minimal')).toHaveText(['100%', '100%']); @@ -239,13 +252,14 @@ test.describe('C24 Probeable Spec', () => { for (const submission of data.submissions) { expect(submission.score).toBe(100); expect(submission.completion_status).toBe('complete'); - expect(submission.variations_count).toBe(5); - // History counts test executions across all 5 variations: 5 x 2 tests. - expect(submission.tests_passed).toBe(10); - expect(submission.total_tests).toBe(10); + // One generated version, graded on both tests; nothing locked. + expect(submission.variations_count).toBe(1); + expect(submission.tests_passed).toBe(2); + expect(submission.total_tests).toBe(2); + expect(submission.locked_count).toBe(0); } // The dropdown shows each attempt's tests_passed/total_tests from history. - await expect(items.locator('.attempt-tests')).toHaveText(['10/10', '10/10']); + await expect(items.locator('.attempt-tests')).toHaveText(['2/2', '2/2']); }); }); diff --git a/purplex/client/src/components/Feedback.vue b/purplex/client/src/components/Feedback.vue index 97bae2c5..9fb89631 100644 --- a/purplex/client/src/components/Feedback.vue +++ b/purplex/client/src/components/Feedback.vue @@ -125,28 +125,10 @@ -
- -

- {{ userPrompt }} -

-
+ :text="userPrompt" + /> +
- - -
@@ -340,7 +256,8 @@ import { defineComponent, nextTick, PropType } from 'vue' import CorrectnessModal from '../modals/CorrectnessModal.vue' import PyTutorModal from '../modals/PyTutorModal.vue' -import SegmentAnalysisModal from './segmentation/SegmentAnalysisModal.vue' +import AbstractionCard from './activities/feedback/AbstractionCard.vue' +import ExplanationQuote from './activities/feedback/ExplanationQuote.vue' import { PythonTutorService } from '@/services/pythonTutor.service' import { formatTestValue } from '@/utils/testValueFormatter' @@ -399,7 +316,6 @@ interface SegmentationProp { interface ComponentData { showCorrectnessModal: boolean - showSegmentAnalysisModal: boolean showPyTutorModal: boolean pythonTutorUrl: string selectedVariant: number @@ -414,9 +330,10 @@ interface ComponentData { export default defineComponent({ components: { + AbstractionCard, CorrectnessModal, - PyTutorModal, - SegmentAnalysisModal + ExplanationQuote, + PyTutorModal }, props: { progress: { @@ -488,7 +405,6 @@ export default defineComponent({ data(): ComponentData { return { showCorrectnessModal: false, - showSegmentAnalysisModal: false, showPyTutorModal: false, pythonTutorUrl: '', selectedVariant: 0, @@ -564,11 +480,6 @@ export default defineComponent({ return !this.allVariationsPass || !this.segmentationEnabled }, - abstractionFill(): number { - if (this.isAbstractionLocked) {return 0} - if (this.segmentCount <= this.segmentThreshold) {return 100} - return Math.max(0, 100 - (this.segmentCount - this.segmentThreshold) * 20) - }, correctnessStatus(): { icon: string; label: string; description: string } { if (this.passingVariants === 0) { @@ -580,15 +491,6 @@ export default defineComponent({ return { icon: '✓', label: this.$t('feedback.correctness.status.clear'), description: this.$t('feedback.correctness.status.clearDesc') } }, - abstractionStatus(): { icon: string; label: string; description: string } { - if (this.isAbstractionLocked) { - return { icon: '🔒', label: this.$t('feedback.abstraction.status.locked'), description: this.$t('feedback.abstraction.status.lockedDesc') } - } - if (this.segmentCount <= this.segmentThreshold) { - return { icon: '✓', label: this.$t('feedback.abstraction.status.highLevel'), description: this.$t('feedback.abstraction.status.highLevelDesc') } - } - return { icon: '✗', label: this.$t('feedback.abstraction.status.tooDetailed'), description: this.$t('feedback.abstraction.status.tooDetailedDesc') } - }, correctnessClass(): string { if (this.passingVariants === 0) {return 'status-error'} @@ -596,11 +498,6 @@ export default defineComponent({ return 'status-success' }, - abstractionClass(): string { - if (this.isAbstractionLocked) {return 'status-locked'} - if (this.segmentCount <= this.segmentThreshold) {return 'status-success'} - return 'status-error' - }, correctnessBarClass(): string { if (this.passingVariants === 0) {return 'bar-error'} @@ -608,11 +505,6 @@ export default defineComponent({ return 'bar-success' }, - abstractionBarClass(): string { - if (this.isAbstractionLocked) {return 'bar-locked'} - if (this.segmentCount <= this.segmentThreshold) {return 'bar-success'} - return 'bar-error' - }, nextStepAction(): string | null { if (this.passingVariants === 0) { @@ -785,14 +677,6 @@ export default defineComponent({ this.announce(this.$t('feedback.announcement.correctnessClosed')) }, - handleAbstractionClick(): void { - if (this.isAbstractionLocked) { - this.announce(this.$t('feedback.announcement.abstractionLocked')) - return - } - this.showSegmentAnalysisModal = true - this.announce(this.$t('feedback.announcement.abstractionOpened')) - }, handleNextStepAction(): void { if (this.passingVariants < this.totalVariants) { @@ -800,7 +684,7 @@ export default defineComponent({ this.openCorrectnessModal() } else if (!this.isAbstractionLocked && this.segmentCount > this.segmentThreshold) { // Open abstraction modal - this.showSegmentAnalysisModal = true + (this.$refs.abstractionCard as InstanceType | undefined)?.open() } else { // Navigate to next problem this.$emit('next-problem') @@ -1188,46 +1072,6 @@ export default defineComponent({ font-size: 10px; } -/* Explanation Section */ -.explanation-section { - margin: var(--spacing-md) var(--spacing-lg); - padding: var(--spacing-md); - background: var(--color-bg-section); - border-radius: var(--radius-base); - border-left: 3px solid var(--color-primary); -} - -.section-label { - display: flex; - align-items: center; - gap: var(--spacing-sm); - margin-bottom: var(--spacing-sm); -} - -.label-icon { - font-size: var(--font-size-base); -} - -.label-text { - font-size: var(--font-size-xs); - font-weight: 600; - color: var(--color-text-muted); - text-transform: uppercase; - letter-spacing: 0.5px; - margin: 0; -} - -.explanation-text { - color: var(--color-text-secondary); - font-size: var(--font-size-sm); - line-height: 1.6; - margin: 0; - padding: var(--spacing-sm); - background: var(--color-bg-panel); - border-radius: var(--radius-xs); - font-style: italic; -} - /* Metric Cards */ .metric-card { display: flex; @@ -1273,16 +1117,6 @@ export default defineComponent({ border-left-color: var(--color-success); } -.metric-card.status-locked { - border-left-color: var(--color-bg-disabled); - opacity: 0.6; - cursor: not-allowed; -} - -.metric-card.status-locked:hover { - background: var(--color-bg-section); -} - .card-label { font-size: 11px; font-weight: 600; @@ -1318,10 +1152,6 @@ export default defineComponent({ background: linear-gradient(90deg, var(--color-success), var(--color-success-dark)); } -.progress-bar-fill.bar-locked { - background: var(--color-bg-border); -} - /* Card Status */ .card-status { display: flex; @@ -1346,11 +1176,6 @@ export default defineComponent({ color: var(--color-text-muted); } -.metric-hint { - color: var(--color-text-muted); - opacity: 0.7; -} - .card-description { font-size: var(--font-size-sm); color: var(--color-text-muted); diff --git a/purplex/client/src/components/__tests__/Feedback.test.ts b/purplex/client/src/components/__tests__/Feedback.test.ts new file mode 100644 index 00000000..edb14aba --- /dev/null +++ b/purplex/client/src/components/__tests__/Feedback.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { mount } from '@vue/test-utils' +import Feedback from '../Feedback.vue' + +const passing = { success: true, testsPassed: 1, totalTests: 1, results: [] } +const failing = { success: false, testsPassed: 0, totalTests: 1, results: [] } + +const segmentation = { + segment_count: 4, + comprehension_level: 'multi_structural', + threshold: 2, + segments: [], +} + +function mountFeedback(testResults: unknown[]) { + return mount(Feedback, { + props: { + codeResults: testResults.map(() => 'def f(x):\n return x'), + testResults: testResults as never, + userPrompt: 'Returns its argument.', + segmentation, + segmentationEnabled: true, + }, + global: { + stubs: { CorrectnessModal: true, PyTutorModal: true, SegmentAnalysisModal: true }, + }, + }) +} + +describe('Feedback (EiPL) explanation and abstraction', () => { + it('quotes the explanation and locks abstraction until every version passes', () => { + const wrapper = mountFeedback([passing, failing]) + + expect(wrapper.find('.explanation-text').text()).toBe('Returns its argument.') + const cards = wrapper.findAll('.metric-card') + expect(cards).toHaveLength(2) + expect(cards[1].classes()).toContain('status-locked') + }) + + it('the next-step button opens the segment analysis when the explanation is too detailed', async () => { + const wrapper = mountFeedback([passing, passing]) + const modal = () => wrapper.findComponent({ name: 'SegmentAnalysisModal' }) + + expect(wrapper.findAll('.metric-card')[1].classes()).toContain('status-error') + expect(modal().props('isVisible')).toBe(false) + + await wrapper.find('.banner-button').trigger('click') + expect(modal().props('isVisible')).toBe(true) + }) +}) diff --git a/purplex/client/src/components/activities/feedback/AbstractionCard.vue b/purplex/client/src/components/activities/feedback/AbstractionCard.vue new file mode 100644 index 00000000..2bf10786 --- /dev/null +++ b/purplex/client/src/components/activities/feedback/AbstractionCard.vue @@ -0,0 +1,352 @@ + + + + + diff --git a/purplex/client/src/components/activities/feedback/CodeSubmissionFeedback.vue b/purplex/client/src/components/activities/feedback/CodeSubmissionFeedback.vue index 83b38199..25947d62 100644 --- a/purplex/client/src/components/activities/feedback/CodeSubmissionFeedback.vue +++ b/purplex/client/src/components/activities/feedback/CodeSubmissionFeedback.vue @@ -134,21 +134,23 @@ - -
-
- {{ $t('feedback.codeSubmission.yourSubmission') }} -
- -
+ + +
+
+ {{ $t('feedback.codeSubmission.yourSubmission') }} +
+ +
+
+ + @@ -300,6 +306,8 @@ interface Props { lockedCount?: number /** Total test count including locked ones (ladder only) */ totalCount?: number | null + /** Show the debugger button on failing tests */ + canDebug?: boolean } const props = withDefaults(defineProps(), { @@ -310,6 +318,7 @@ const props = withDefaults(defineProps(), { layout: 'groups', lockedCount: 0, totalCount: null, + canDebug: true, }) const emit = defineEmits<{ diff --git a/purplex/client/src/components/activities/feedback/ExplanationQuote.vue b/purplex/client/src/components/activities/feedback/ExplanationQuote.vue new file mode 100644 index 00000000..d0ccd572 --- /dev/null +++ b/purplex/client/src/components/activities/feedback/ExplanationQuote.vue @@ -0,0 +1,77 @@ + + + + + diff --git a/purplex/client/src/components/activities/feedback/ProbeableCodeFeedback.vue b/purplex/client/src/components/activities/feedback/ProbeableCodeFeedback.vue deleted file mode 100644 index f4bcbfda..00000000 --- a/purplex/client/src/components/activities/feedback/ProbeableCodeFeedback.vue +++ /dev/null @@ -1,127 +0,0 @@ - - - diff --git a/purplex/client/src/components/activities/feedback/ProbeableFeedback.vue b/purplex/client/src/components/activities/feedback/ProbeableFeedback.vue new file mode 100644 index 00000000..9703e476 --- /dev/null +++ b/purplex/client/src/components/activities/feedback/ProbeableFeedback.vue @@ -0,0 +1,160 @@ + + + diff --git a/purplex/client/src/components/activities/feedback/ProbeableSpecFeedback.vue b/purplex/client/src/components/activities/feedback/ProbeableSpecFeedback.vue deleted file mode 100644 index fd24fcf6..00000000 --- a/purplex/client/src/components/activities/feedback/ProbeableSpecFeedback.vue +++ /dev/null @@ -1,89 +0,0 @@ - - - diff --git a/purplex/client/src/components/activities/feedback/TestLadder.vue b/purplex/client/src/components/activities/feedback/TestLadder.vue index d4d65d05..1c03d99a 100644 --- a/purplex/client/src/components/activities/feedback/TestLadder.vue +++ b/purplex/client/src/components/activities/feedback/TestLadder.vue @@ -88,6 +88,7 @@ :aria-label="$t('feedback.tests.pipFailed', { n: i + 1 })" >✗