src/plugins/trackers/builtin/json.ts (2)

403-414: Consider extracting duplicated error handling logic.

The PrdJsonSchemaError handling block is duplicated in both getTasks (lines 403-412) and getEpics (lines 615-624). This could be extracted to a private helper method for consistency and maintainability.

♻️ Suggested refactor

private logPrdSchemaError(err: PrdJsonSchemaError): void {
  console.error(`\n${err.message}\n`);
  console.error('Issues found:');
  for (const detail of err.details) {
    console.error(`  - ${detail}`);
  }
  console.error(`\nHow to fix:\n${err.suggestion}\n`);
}

Then use in both methods:

    } catch (err) {
      if (err instanceof PrdJsonSchemaError) {
-       console.error(`\n${err.message}\n`);
-       console.error('Issues found:');
-       for (const detail of err.details) {
-         console.error(`  - ${detail}`);
-       }
-       console.error(`\nHow to fix:\n${err.suggestion}\n`);
+       this.logPrdSchemaError(err);
      } else {

Also applies to: 615-626

---

84-93: Optional: Consider adding prototype chain safeguard for custom Error subclass.

With your current ES2022 transpilation target, the instanceof checks for PrdJsonSchemaError work correctly without additional fixes. However, if the codebase is transpiled to older JavaScript versions (ES5 or early ES2015), the prototype chain may not be correctly set, causing instanceof checks to fail. Adding Object.setPrototypeOf is a defensive measure that remains harmless in modern environments while future-proofing against transpilation target changes.

🔧 Optional safeguard

export class PrdJsonSchemaError extends Error {
  constructor(
    message: string,
    public readonly details: string[],
    public readonly suggestion: string
  ) {
    super(message);
    this.name = 'PrdJsonSchemaError';
+    Object.setPrototypeOf(this, PrdJsonSchemaError.prototype);
  }
}

src/commands/convert.ts (1)

508-520: Validation placement is correct, but consider the error message context.

The pre-write validation is a good safety net. However, if validation fails here, it indicates a bug in the internal convertToPrdJson function (since the PRD parser should produce valid structures). The error message from PrdJsonSchemaError.suggestion mentions "regenerate with convert command", which would be confusing since this is the convert command.

Consider adding context-specific messaging for this case:

🔧 Suggested improvement

   try {
     validatePrdJsonSchema(prdJson, outputPath);
   } catch (err) {
     if (err instanceof PrdJsonSchemaError) {
-      printError('Generated prd.json failed schema validation:');
+      printError('Internal error: Generated prd.json failed schema validation.');
+      printError('This indicates a bug in the PRD parser. Please report this issue.');
       for (const detail of err.details) {
         console.error(`  - ${detail}`);
       }
-      console.error(`\n${err.suggestion}`);
+      // Don't print the generic suggestion as it refers to re-running this command
       process.exit(1);
     }
     throw err;
   }

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/plugins/trackers/builtin/json.ts (3)

190-198: Minor: Potential duplicate error message for status field.

When a story has a status field but no passes field, the user receives two error messages: one from the passes validation (lines 180-184) and another from the unsupported fields check here. Consider removing 'status' from unsupportedFields since it's already handled with a more specific message above.

🔧 Suggested fix

-      const unsupportedFields = ['subtasks', 'estimated_hours', 'files', 'status'];
+      const unsupportedFields = ['subtasks', 'estimated_hours', 'files'];

---

221-221: Consider validating metadata fields.

The metadata object is cast directly without validating its internal structure. If an AI hallucinates incorrect types for createdAt, updatedAt, or version (e.g., numbers instead of strings), they would pass through. This is low-risk since metadata is optional, but could cause subtle issues if downstream code assumes string types.

🔧 Optional validation

-    metadata: obj.metadata as PrdJson['metadata'],
+    metadata: obj.metadata && typeof obj.metadata === 'object'
+      ? {
+          createdAt: typeof (obj.metadata as Record<string, unknown>).createdAt === 'string'
+            ? (obj.metadata as Record<string, unknown>).createdAt as string
+            : undefined,
+          updatedAt: typeof (obj.metadata as Record<string, unknown>).updatedAt === 'string'
+            ? (obj.metadata as Record<string, unknown>).updatedAt as string
+            : undefined,
+          version: typeof (obj.metadata as Record<string, unknown>).version === 'string'
+            ? (obj.metadata as Record<string, unknown>).version as string
+            : undefined,
+        }
+      : undefined,

---

403-412: Consider extracting duplicate error handling to a helper.

The error formatting logic for PrdJsonSchemaError is duplicated in both getTasks (lines 403-412) and getEpics (lines 615-624). If the error format needs to change, both locations must be updated.

🔧 Suggested refactor

Add a method to PrdJsonSchemaError or a standalone helper:

// Option 1: Add to PrdJsonSchemaError class
printError(): void {
  console.error(`\n${this.message}\n`);
  console.error('Issues found:');
  for (const detail of this.details) {
    console.error(`  - ${detail}`);
  }
  console.error(`\nHow to fix:\n${this.suggestion}\n`);
}

Then simplify the catch blocks:

     } catch (err) {
       if (err instanceof PrdJsonSchemaError) {
-        console.error(`\n${err.message}\n`);
-        console.error('Issues found:');
-        for (const detail of err.details) {
-          console.error(`  - ${detail}`);
-        }
-        console.error(`\nHow to fix:\n${err.suggestion}\n`);
+        err.printError();
       } else {

Also applies to: 615-624

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/plugins/trackers/builtin/json.ts (1)

229-234: Misleading comment and unvalidated metadata cast.

Two minor observations:

1. The comment on line 231 mentions "convert from status if needed as fallback", but status conversion isn't actually performed here—the fallback simply defaults to false.

2. obj.metadata is cast directly without structural validation. If the metadata contains unexpected types (e.g., non-string values for createdAt), downstream code might encounter type mismatches.

Suggested improvements

     userStories: (obj.userStories as PrdUserStory[]).map((s) => ({
       ...s,
-      // Ensure passes is a boolean (convert from status if needed as fallback)
+      // Ensure passes is a boolean (default to false if somehow missing after validation)
       passes: typeof s.passes === 'boolean' ? s.passes : false,
     })),
-    metadata: obj.metadata as PrdJson['metadata'],
+    metadata: typeof obj.metadata === 'object' && obj.metadata !== null
+      ? (obj.metadata as PrdJson['metadata'])
+      : undefined,

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/tui/components/PrdChatApp.tsx (1)

87-126: Schema example includes fields beyond what is listed as required, potentially confusing the AI.

The jsonSchemaExample (lines 87-101) includes description, acceptanceCriteria, and priority fields, but the "Required fields" section (lines 113-117) only lists id, title, passes, and dependsOn. Additionally, the PR objective from issue #11 specifies the expected schema as:

{ name, branchName, userStories: [ { id, title, passes, dependsOn } ] }

This mismatch may lead to inconsistent AI output — the model might include the extra fields from the example or omit them based on the required list. Consider either:

1. Aligning the example to only show the truly required fields, or
2. Expanding the "Required fields" list to explicitly document which fields are optional.

♻️ Suggested alignment with issue #11's expected schema

   const jsonSchemaExample = `{
   "name": "Feature Name",
   "branchName": "feature/my-feature",
   "userStories": [
     {
       "id": "US-001",
       "title": "Story title",
-      "description": "As a user, I want...",
-      "acceptanceCriteria": ["Criterion 1"],
-      "priority": 1,
       "passes": false,
       "dependsOn": []
     }
   ]
 }`;

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro