Skip to content

Improve problem generation quality and verification UI#12

Merged
smilebank7 merged 1 commit into
mainfrom
codex/problem-generation-quality-ui-fixes
Jun 6, 2026
Merged

Improve problem generation quality and verification UI#12
smilebank7 merged 1 commit into
mainfrom
codex/problem-generation-quality-ui-fixes

Conversation

@korearororo

@korearororo korearororo commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add deterministic structural/conceptual problem templates and guards across middle-school topics
  • Improve generation kind handling, SymPy extraction/equivalence, objective mapping, and independent re-solve robustness
  • Add quality audit, judge, and sweep scripts with broad regression coverage
  • Fix verification UI startup/error handling by loading packages/agent/.env from the monorepo root and surfacing pre-yield workflow errors through SSE
  • Fix result/export navigation so adopted problems and back links preserve query context

Verification

  • pnpm -F @openmath/agent test -- env backend-contract
  • pnpm typecheck
  • Pre-push hook: pnpm test
    • @openmath/agent: 137 tests passed
    • math-engine: 19 tests passed
  • Browser QA: verify progress UI updates through 6 steps; result adoption/PDF/export/back-link flow works

Summary by cubic

Improves problem generation quality with deterministic templates, topic-guard rails, and stronger SymPy/equivalence checks; also fixes verification UI startup and error surfacing while preserving navigation context.

  • New Features

    • Deterministic structural/conceptual templates by topic with guard hints; adds generation_kind mapped from topic and passed through prompts/schemas.
    • Stronger verification: equation extractor and LaTeX→SymPy normalizer, broader SymPy checks, and robust answer equivalence (choices, labels, geometry multi-values, fractions/repeating decimals, graphs).
    • Objective mapping adds deterministic checks and topic-kind enforcement; independent re-solve compares via new equivalence; constraint critic recovers from raw JSON text.
    • RAG search prioritizes exact topic, then relaxes; supports achievement_code; seeds a deterministic candidate when refs exist.
    • Quality tooling: scripts/sweep-generate.mjs, scripts/audit-quality.ts, packages/agent/scripts/judge-quality.ts; adds pnpm sweep:generate; broad test coverage in @openmath/agent.
  • Bug Fixes

    • @openmath/agent loads .env from the monorepo root; startup is more reliable.
    • SSE now surfaces pre-yield workflow errors and includes gate failure details in step summaries.
    • @openmath/web result/export views preserve grade/topic/mode/dims/source in links and storage keys; show a helpful error if the stream ends with no results.

Written for commit bb2c885. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

28 issues found across 58 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/agent/prompts/constraint-critic.md">

<violation number="1" location="packages/agent/prompts/constraint-critic.md:42">
P2: Prompt behavior changed without bumping the prompt version. This breaks the repo’s prompt-versioning contract and can blur generation metadata/tracking.</violation>
</file>

<file name="packages/agent/prompts/independent-solver.md">

<violation number="1" location="packages/agent/prompts/independent-solver.md:24">
P3: Prompt content changed without bumping the prompt version, so downstream prompt metadata will not reflect this new behavior.</violation>
</file>

<file name="packages/agent/src/steps/independent-resolve.ts">

<violation number="1" location="packages/agent/src/steps/independent-resolve.ts:53">
P2: Do not echo raw answer text into `failure_detail.message`; this field is reused as a refinement hint and can contaminate later prompts. Keep the mismatch message generic and log the detailed answers separately.</violation>
</file>

<file name="packages/agent/src/agents/constraint-critic-agent.ts">

<violation number="1" location="packages/agent/src/agents/constraint-critic-agent.ts:71">
P2: Backslash-repair regex over-escapes already-valid `\\` sequences, which can turn valid critique JSON into invalid JSON and break fallback parsing.</violation>
</file>

<file name="packages/agent/src/steps/problem-generation-deterministic.ts">

<violation number="1" location="packages/agent/src/steps/problem-generation-deterministic.ts:17">
P3: Duplicate candidate-construction logic already exists in `GeneratorAgent`; extract a shared helper to keep the `GeneratedProblem` mapping in one place.</violation>
</file>

<file name="packages/agent/src/steps/sympy-verification.ts">

<violation number="1" location="packages/agent/src/steps/sympy-verification.ts:73">
P1: Choice-style equation candidates bypass SymPy solving and pass on non-empty text alone.</violation>

<violation number="2" location="packages/agent/src/steps/sympy-verification.ts:83">
P1: Expression candidates are marked passed without using SymPy to decide correctness.</violation>
</file>

<file name="packages/agent/src/schemas/generated-problem.schema.ts">

<violation number="1" location="packages/agent/src/schemas/generated-problem.schema.ts:36">
P3: Adding a required `generation_kind` changes the public `GeneratedProblem`/SSE result contract, but the domain spec still describes the old shape. Please update the spec and any contract fixtures together to keep consumers and source-of-truth aligned.</violation>
</file>

<file name="packages/agent/src/tools/answer-equivalence.ts">

<violation number="1" location="packages/agent/src/tools/answer-equivalence.ts:42">
P0: Equation answers are accepted based on the expected answer alone, so the independent solver output is never validated for equation candidates.</violation>

<violation number="2" location="packages/agent/src/tools/answer-equivalence.ts:43">
P1: Repeating-decimal questions short-circuit on the expected answer only, so the re-solve result is not actually compared against the solver output.</violation>
</file>

<file name="packages/agent/src/tools/equation-extractor.ts">

<violation number="1" location="packages/agent/src/tools/equation-extractor.ts:1">
P2: Equation extraction can include list-number prefixes like `1.` and return an invalid equation string.</violation>
</file>

<file name="packages/web/app/app/new/export/view.tsx">

<violation number="1" location="packages/web/app/app/new/export/view.tsx:76">
P2: Back links from export to result drop the adopted selection context, so returning to results loses the starred choices and the PDF flow cannot be resumed cleanly.</violation>
</file>

<file name="packages/agent/src/tools/rag-client.ts">

<violation number="1" location="packages/agent/src/tools/rag-client.ts:349">
P2: `tokensMatch()` became asymmetric, so longer query tokens no longer match shorter corpus tokens.</violation>
</file>

<file name="packages/agent/src/steps/objective-mapping.ts">

<violation number="1" location="packages/agent/src/steps/objective-mapping.ts:152">
P2: The topic-mismatch bypass is too weak. Any single token overlap with `topic_name`, `source_problem_text`, or a broad alias can suppress `intent_topic_mismatch`, so off-topic candidates can slip through the deterministic objective gate.</violation>
</file>

<file name="packages/agent/prompts/problem-generator.md">

<violation number="1" location="packages/agent/prompts/problem-generator.md:26">
P3: Bump the prompt version when changing this prompt body; leaving `version` at 0.1.0 makes the prompt metadata stale.</violation>

<violation number="2" location="packages/agent/prompts/problem-generator.md:81">
P2: Declare `generationKind` in the prompt variables list; otherwise the template contract is incomplete and the new placeholder can render empty or be missed by callers/tests.</violation>
</file>

<file name="scripts/audit-quality.ts">

<violation number="1" location="scripts/audit-quality.ts:92">
P2: Loading the topic source via regex and hard-coding `30` makes the audit brittle to formatting or data-set changes.</violation>

<violation number="2" location="scripts/audit-quality.ts:200">
P2: The topic-evidence check is too permissive: generic single-character and operator aliases make the audit easy to satisfy without real topic-specific evidence.</violation>
</file>

<file name="packages/agent/src/steps/problem-generation-deterministic-templates.ts">

<violation number="1" location="packages/agent/src/steps/problem-generation-deterministic-templates.ts:43">
P2: Avoid commas inside this expected answer; they will be treated as answer delimiters and make verification brittle.</violation>

<violation number="2" location="packages/agent/src/steps/problem-generation-deterministic-templates.ts:48">
P2: Use the canonical topic-code resolver here; prioritizing `topic` over `topic_code` can select the wrong deterministic template when both fields are present.</violation>
</file>

<file name="packages/agent/src/tools/math-engine-client.ts">

<violation number="1" location="packages/agent/src/tools/math-engine-client.ts:128">
P1: Route fraction-containing expressions through `toSympyInput` only after fixing exponent precedence; powered fractions are currently serialized with the wrong meaning.</violation>
</file>

<file name="packages/agent/src/tools/sympy-input.ts">

<violation number="1" location="packages/agent/src/tools/sympy-input.ts:17">
P2: Do not blindly insert `*` between digits and letters; it corrupts scientific-notation literals such as `1e3` before SymPy can parse them.</violation>

<violation number="2" location="packages/agent/src/tools/sympy-input.ts:21">
P1: Do not force `*` between a name and `(`; it breaks spaced function calls that SymPy can already parse.</violation>
</file>

<file name="scripts/sweep-generate.mjs">

<violation number="1" location="scripts/sweep-generate.mjs:107">
P2: Use strict numeric parsing here; `parseInt` will accept partially valid strings and can run the sweep with unintended concurrency/repeat/timeout values.</violation>
</file>

<file name="packages/agent/src/tools/repeating-decimal.ts">

<violation number="1" location="packages/agent/src/tools/repeating-decimal.ts:42">
P2: Repeating-decimal detection is capped at 4 digits, so longer valid repetends will not be recognized and equivalence checks will fail for those questions.</violation>
</file>

<file name="packages/agent/scripts/judge-quality.ts">

<violation number="1" location="packages/agent/scripts/judge-quality.ts:181">
P1: Judge parse/runtime failures are downgraded to WARN, so a broken audit run can still exit 0 and look successful.</violation>
</file>

<file name="packages/agent/src/steps/problem-generation.ts">

<violation number="1" location="packages/agent/src/steps/problem-generation.ts:183">
P2: Choice-marker detection is too broad and misclassifies decimal equations as multiple-choice, skipping expected-answer normalization.</violation>
</file>

<file name="packages/agent/src/steps/rag-search.ts">

<violation number="1" location="packages/agent/src/steps/rag-search.ts:80">
P1: Empty `topic_name` values are treated as strict topic filters and can incorrectly discard all RAG results.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

}

if (sameNormalizedSet(expected, derived)) return true;
if (await expectedMatchesEquationSolve(mathEngine, candidate, expected)) return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0: Equation answers are accepted based on the expected answer alone, so the independent solver output is never validated for equation candidates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/tools/answer-equivalence.ts, line 42:

<comment>Equation answers are accepted based on the expected answer alone, so the independent solver output is never validated for equation candidates.</comment>

<file context>
@@ -0,0 +1,277 @@
+  }
+
+  if (sameNormalizedSet(expected, derived)) return true;
+  if (await expectedMatchesEquationSolve(mathEngine, candidate, expected)) return true;
+  if (expectedMatchesRepeatingDecimalQuestion(candidate, expected)) return true;
+
</file context>

};
}

if (candidate.generation_kind === "expression") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Expression candidates are marked passed without using SymPy to decide correctness.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/steps/sympy-verification.ts, line 83:

<comment>Expression candidates are marked passed without using SymPy to decide correctness.</comment>

<file context>
@@ -60,12 +66,66 @@ export async function verifyWithSympy(
+    };
+  }
+
+  if (candidate.generation_kind === "expression") {
+    await verifyExpressionAnswer(mathEngine, candidate.expected_answer);
+  }
</file context>

if (candidate.generation_kind === "equation") {
if (isChoiceStyleCandidate(candidate)) {
return {
passed: candidate.question_text.trim().length > 0 && candidate.expected_answer.trim().length > 0,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Choice-style equation candidates bypass SymPy solving and pass on non-empty text alone.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/steps/sympy-verification.ts, line 73:

<comment>Choice-style equation candidates bypass SymPy solving and pass on non-empty text alone.</comment>

<file context>
@@ -60,12 +66,66 @@ export async function verifyWithSympy(
+  if (candidate.generation_kind === "equation") {
+    if (isChoiceStyleCandidate(candidate)) {
+      return {
+        passed: candidate.question_text.trim().length > 0 && candidate.expected_answer.trim().length > 0,
+        verificationKind: candidate.generation_kind,
+      };
</file context>


if (sameNormalizedSet(expected, derived)) return true;
if (await expectedMatchesEquationSolve(mathEngine, candidate, expected)) return true;
if (expectedMatchesRepeatingDecimalQuestion(candidate, expected)) return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Repeating-decimal questions short-circuit on the expected answer only, so the re-solve result is not actually compared against the solver output.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/tools/answer-equivalence.ts, line 43:

<comment>Repeating-decimal questions short-circuit on the expected answer only, so the re-solve result is not actually compared against the solver output.</comment>

<file context>
@@ -0,0 +1,277 @@
+
+  if (sameNormalizedSet(expected, derived)) return true;
+  if (await expectedMatchesEquationSolve(mathEngine, candidate, expected)) return true;
+  if (expectedMatchesRepeatingDecimalQuestion(candidate, expected)) return true;
+
+  const expectedCanonical = await tryCanonicalizeAll(mathEngine, expected);
</file context>

verify: (req) => post("/verify", req, parseVerifyResponse),
simplify: (req) => post("/simplify", req, parseSimplifyResponse),
solve: (req) =>
post("/solve", { ...req, equation: toSympyInput(req.equation) }, parseSolveResponse),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Route fraction-containing expressions through toSympyInput only after fixing exponent precedence; powered fractions are currently serialized with the wrong meaning.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/tools/math-engine-client.ts, line 128:

<comment>Route fraction-containing expressions through `toSympyInput` only after fixing exponent precedence; powered fractions are currently serialized with the wrong meaning.</comment>

<file context>
@@ -122,11 +124,22 @@ export function createMathEngineClient(
-    verify: (req) => post("/verify", req, parseVerifyResponse),
-    simplify: (req) => post("/simplify", req, parseSimplifyResponse),
+    solve: (req) =>
+      post("/solve", { ...req, equation: toSympyInput(req.equation) }, parseSolveResponse),
+    verify: (req) =>
+      post(
</file context>

}

function hasChoiceMarkers(text: string): boolean {
return /[①②③④⑤⑥⑦⑧⑨]|\([1-9]\)|[1-9][).]/u.test(text);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Choice-marker detection is too broad and misclassifies decimal equations as multiple-choice, skipping expected-answer normalization.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/steps/problem-generation.ts, line 183:

<comment>Choice-marker detection is too broad and misclassifies decimal equations as multiple-choice, skipping expected-answer normalization.</comment>

<file context>
@@ -133,11 +166,30 @@ async function normalizeExpectedAnswer(
 }
+
+function hasChoiceMarkers(text: string): boolean {
+  return /[①②③④⑤⑥⑦⑧⑨]|\([1-9]\)|[1-9][).]/u.test(text);
+}
+
</file context>
Suggested change
return /[]|\([1-9]\)|[1-9][).]/u.test(text);
return /[]|\([1-9]\)|(?:^|\s)[1-9][).](?=\s|$)/u.test(text);

@@ -19,6 +19,10 @@ updated: 2026-05-18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Prompt content changed without bumping the prompt version, so downstream prompt metadata will not reflect this new behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/prompts/independent-solver.md, line 24:

<comment>Prompt content changed without bumping the prompt version, so downstream prompt metadata will not reflect this new behavior.</comment>

<file context>
@@ -19,6 +19,10 @@ updated: 2026-05-18
 
+# Generation Kind
+
+{{candidate.generation_kind}}
+
 # Task
</file context>

@@ -0,0 +1,32 @@
import { randomUUID } from "node:crypto";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Duplicate candidate-construction logic already exists in GeneratorAgent; extract a shared helper to keep the GeneratedProblem mapping in one place.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/steps/problem-generation-deterministic.ts, line 17:

<comment>Duplicate candidate-construction logic already exists in `GeneratorAgent`; extract a shared helper to keep the `GeneratedProblem` mapping in one place.</comment>

<file context>
@@ -0,0 +1,32 @@
+  const template = deterministicTemplateForRequest(input.request);
+  if (template === null) return null;
+  return {
+    candidate_id: randomUUID(),
+    mode: input.request.mode === "conceptual" ? "conceptual" : "structural",
+    generation_kind: generationKindForTopic(topicCode),
</file context>

@@ -12,6 +12,7 @@

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Adding a required generation_kind changes the public GeneratedProblem/SSE result contract, but the domain spec still describes the old shape. Please update the spec and any contract fixtures together to keep consumers and source-of-truth aligned.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/src/schemas/generated-problem.schema.ts, line 36:

<comment>Adding a required `generation_kind` changes the public `GeneratedProblem`/SSE result contract, but the domain spec still describes the old shape. Please update the spec and any contract fixtures together to keep consumers and source-of-truth aligned.</comment>

<file context>
@@ -32,6 +33,7 @@ export type GenerationMeta = z.infer<typeof GenerationMetaSchema>;
 export const GeneratedProblemSchema = z.object({
   candidate_id: z.string().uuid(),
   mode: IsomorphicModeSchema,
+  generation_kind: GenerationKindSchema,
 
   question_text: z.string().min(1), // LaTeX
</file context>

@@ -23,6 +23,26 @@ updated: 2026-05-18

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: Bump the prompt version when changing this prompt body; leaving version at 0.1.0 makes the prompt metadata stale.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/agent/prompts/problem-generator.md, line 26:

<comment>Bump the prompt version when changing this prompt body; leaving `version` at 0.1.0 makes the prompt metadata stale.</comment>

<file context>
@@ -23,6 +23,26 @@ updated: 2026-05-18
 
 `{{request.mode}}` — `structural` 이면 같은 풀이법, 다른 숫자/표현. `conceptual` 이면 같은 학습 목표·평가 차원, 다른 풀이 경로.
 
+# Generation Kind
+
+`{{generationKind}}`
</file context>

@smilebank7 smilebank7 merged commit b8f10d0 into main Jun 6, 2026
5 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