fix(audit): align schema pattern detection#50
Conversation
|
@KimHyeongRae0 is attempting to deploy a commit to the Cytonic Team on Vercel. A member of the Team first needs to authorize it. |
fb7823c to
0663375
Compare
|
Strong fix @KimHyeongRae0 — the audit was using regex shortcuts to detect FAQ/HowTo while the schema generator used proper detection logic, so the audit could report "FAQPage schema eligible" while the schema generator would silently produce nothing. Extracting Three new tests covering the three cases (FAQ without answer, single-step "HowTo", real FAQ) make the regression boundary clear. Approving. Will merge once the open queue clears. |
|
Thanks. Marked this ready for review. |
Greptile SummaryThis PR eliminates the drift between how
Confidence Score: 4/5Safe to merge; the refactoring faithfully replicates the original detection logic in both consumers, and the intentional tightening of the audit criteria aligns it with what schema generation actually produces. The only gap is a missing positive test for the HowTo 2-step path — if a future change accidentally breaks the steps.length >= 2 guard or the step regex, no test would catch it on the audit side. src/core/audit.test.ts — the HowTo passing path is the one area that would benefit from an additional test case. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[auditSite] --> B[auditSchemaPresence]
A --> C[auditCitability]
B --> D{any page matches?}
D -->|FAQ path| E["detectFaqPatterns\nquestion-word heading + answer text"]
D -->|HowTo path| F["detectHowToSteps\nnumbered steps, min 2 required"]
E --> G[hasFaqOrHowTo]
F --> G
C --> H{any page matches?}
H -->|FAQ path| I["detectFaqPatterns\nsame shared function"]
I --> J[hasFaq]
subgraph SP["schema-patterns.ts (shared)"]
E
F
I
end
OLD1["audit.ts old: broad regex\nany heading ending with ?"] -."replaced by".-> E
OLD2["audit.ts old: How to heading\nor Step N heading"] -."replaced by".-> F
OLD3["schema.ts old: private fns\nidentical logic"] -."extracted to".-> SP
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/core/audit.test.ts:111-135
**Missing positive HowTo test case**
The three new tests cover: a non-question heading that should fail, a single-step heading that should fail, and a valid FAQ that should pass — but there is no test asserting that content with **two or more step headings** produces `passed: true`. Because `detectHowToSteps` returns `[]` for fewer than 2 steps, the only way to confirm the HowTo pass path works is to test it with at least two `## Step N:` headings. Without this, a regression in the `>= 2` guard or the step regex could silently go undetected.
Reviews (1): Last reviewed commit: "fix(audit): align schema pattern detecti..." | Re-trigger Greptile |
| it('requires at least two HowTo steps when auditing schema presence', () => { | ||
| const config = makeConfig({ | ||
| pages: [{ | ||
| pathname: '/install', | ||
| content: '## Step 1: Install\n\nRun npm install aeo.js.', | ||
| }], | ||
| }); | ||
| const result = auditSite(config); | ||
| const schemaPresence = result.categories.find(c => c.name === 'Schema Presence')!; | ||
| const faqOrHowToCheck = schemaPresence.checks.find(c => c.label === 'FAQPage or HowTo schema'); | ||
| expect(faqOrHowToCheck?.passed).toBe(false); | ||
| }); | ||
|
|
||
| it('passes schema presence for FAQ content that can generate FAQPage schema', () => { | ||
| const config = makeConfig({ | ||
| pages: [{ | ||
| pathname: '/faq', | ||
| content: '## What is aeo.js?\n\naeo.js generates answer-engine assets for websites.', | ||
| }], | ||
| }); | ||
| const result = auditSite(config); | ||
| const schemaPresence = result.categories.find(c => c.name === 'Schema Presence')!; | ||
| const faqOrHowToCheck = schemaPresence.checks.find(c => c.label === 'FAQPage or HowTo schema'); | ||
| expect(faqOrHowToCheck?.passed).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Missing positive HowTo test case
The three new tests cover: a non-question heading that should fail, a single-step heading that should fail, and a valid FAQ that should pass — but there is no test asserting that content with two or more step headings produces passed: true. Because detectHowToSteps returns [] for fewer than 2 steps, the only way to confirm the HowTo pass path works is to test it with at least two ## Step N: headings. Without this, a regression in the >= 2 guard or the step regex could silently go undetected.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/audit.test.ts
Line: 111-135
Comment:
**Missing positive HowTo test case**
The three new tests cover: a non-question heading that should fail, a single-step heading that should fail, and a valid FAQ that should pass — but there is no test asserting that content with **two or more step headings** produces `passed: true`. Because `detectHowToSteps` returns `[]` for fewer than 2 steps, the only way to confirm the HowTo pass path works is to test it with at least two `## Step N:` headings. Without this, a regression in the `>= 2` guard or the step regex could silently go undetected.
How can I resolve this? If you propose a fix, please make it concise.
Fixes audit/schema FAQ and HowTo detection drift. Validation:
npm run lint,npm run test -- --run,npm run build.