Skip to content

Add doc comments for widget *PromptJSON types#3454

Open
benchristel wants to merge 28 commits intomainfrom
benc/prompt-json-docs
Open

Add doc comments for widget *PromptJSON types#3454
benchristel wants to merge 28 commits intomainfrom
benc/prompt-json-docs

Conversation

@benchristel
Copy link
Copy Markdown
Member

@benchristel benchristel commented Apr 2, 2026

These doc comments will appear in the Perseus API docs. The intended
audience is engineers on other KA teams who are feeding Perseus user
input data to LLMs.

Issue: LEMS-3893

Test plan:

None

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

Size Change: 0 B

Total Size: 497 kB

ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.5 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 6.36 kB
packages/math-input/dist/es/index.js 98.5 kB
packages/math-input/dist/es/strings.js 1.61 kB
packages/perseus-core/dist/es/index.item-splitting.js 12 kB
packages/perseus-core/dist/es/index.js 25.2 kB
packages/perseus-editor/dist/es/index.js 102 kB
packages/perseus-linter/dist/es/index.js 9.3 kB
packages/perseus-score/dist/es/index.js 9.7 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/index.js 194 kB
packages/perseus/dist/es/strings.js 8.27 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (15b2af7) and published it to npm. You
can install it using the tag PR3454.

Example:

pnpm add @khanacademy/perseus@PR3454

If you are working in Khan Academy's frontend, you can run the below command.

./dev/tools/bump_perseus_version.ts -t PR3454

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR3454

@benchristel benchristel marked this pull request as ready for review April 6, 2026 23:32
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

export type {CategorizerPromptJSON} from "./widget-ai-utils/categorizer/categorizer-ai-utils";
export type {DefinitionPromptJSON} from "./widget-ai-utils/definition/definition-ai-utils";
export type {DropdownPromptJSON} from "./widget-ai-utils/dropdown/dropdown-ai-utils";
export type {UnsupportedWidgetPromptJSON} from "./widget-ai-utils/unsupported-widget";
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I need to add exports for the other types here!

@anakaren-rojas
Copy link
Copy Markdown
Contributor

I wonder if we also need to add a widget guidance to the comments?
Basically: use X widget instead of Y widget.
Like use numeric input instead of input number

Comment on lines +31 to +32
* The current state of the widget user interface. Usually represents a
* learner's attempt to answer a question.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm trying to think... but are there situations where this isn't the learner's attempt?

Comment on lines +36 to +40
* The category indices for each item. Elements in this array
* correspond to elements of `options.items`, and refer to indices of
* the `categories` array. For example, a value of `[2, null, 0]` means
* that the first item is in category 2, the second item has not been
* assigned to a category, and the third item is in category 0.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The first item in the array (2) means the third category was selected, right (they're 0-based array indices). It becomes clear when I read to the end and see that the "third item is in category 0" but it is slightly confusing for me.

hasNoneOfTheAbove: boolean;

/**
* The answer choices presented to the learner.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are these guaranteed to be in the order that the learner saw them?

Comment on lines -11 to -13
type BaseGraphOptions = {
type: string;
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for migrating these types to embed a constant string for each type! That's a great improvement.

import type React from "react";

/**
* JSON describing an InputNumber widget. Intended for consumption by AI tools.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if there's benefits, but perhaps we describe this widget as being deprecated and that numeric-input is preferred?

// TODO(LEMS-4033): render an intelligible string for simplify; the
// current values ("optional", "required", "enforced") aren't
// self-explanatory.
simplify: string;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not directly related, but can this be a union of the actual string values this can be? It is for the main widget options:

simplify: "required" | "optional" | "enforced";

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants