docs: Restructure AGENTS.md for better agent guidance and progressive disclosure#123
docs: Restructure AGENTS.md for better agent guidance and progressive disclosure#123bensonwong merged 2 commits intomainfrom
Conversation
…ges addressing all 33 feedback items: ## Changes Made **Structure & Progressive Disclosure (Feedback #1-5, #29-30):** - Added "Agent Behavior Guidelines" at the very top - Added "Is DeepCitation Right for You?" quick check - Moved Critical Warnings to prominent position with visual examples - Moved Quick Start complete example to near-top - Added Core Workflow ASCII diagram with "Why" explanations - Reorganized by user journey (basics → advanced) **Preventing Fabrication (Feedback #6-7, #18-19):** - Added explicit DO/DON'T list for agents - Created Appendix A: Real URLs (complete list of all legitimate URLs) - Created Appendix B: What DeepCitation Doesn't Do **Critical Warnings (Feedback #11-13):** - Made `<<<CITATION_DATA>>>` warning extremely prominent with visual before/after - Added streaming guidance about when the block arrives - Added security notes throughout (API key handling, unsafe URL mode) **Technical Completeness (Feedback #8, #14-17, #24-28):** - Added error handling patterns with try/catch - Explained async nature of verification - Added `prepareUrl()` method with 30s processing time warning - Added `unsafeFastUrlOutput` security warning - Expanded multiple documents pattern with explicit `fileDataParts` mapping - Added UI guidance to verification status table **Troubleshooting (Feedback #10, #32):** - Expanded each issue with: Symptoms, Causes, Solutions, DON'T sections - Separated from Agent Guidelines **Collapsible Sections (Feedback #3, #31):** - Used `<details>` for LLM provider alternatives - Used `<details>` for React component variants - Used `<details>` for unsafe URL mode **Reference Sections (Feedback #20-23, #25):** - Added descriptions to Key Imports Reference - Added Production Checklist in prominent location - Moved Next.js API routes under Framework Integration
✅ Playwright Test ReportStatus: Tests passed 📊 View Full Report What's in the Visual SnapshotsThe gallery includes visual snapshots for:
Run ID: 21539096352 | Download Artifacts |
Pull Request Review: AGENTS.md ImprovementsSummaryThis PR significantly improves AGENTS.md by restructuring it to be more agent-friendly and addressing 33 specific feedback items. The changes focus on preventing AI agents from fabricating URLs/methods, improving progressive disclosure, and providing clearer examples. Strengths1. Excellent Agent Guardrails ✅The new "Agent Behavior Guidelines" section at the top directly addresses the fabrication problem mentioned in the PR description. 2. Critical Warning Prominence ✅The 3. Progressive Disclosure ✅Much better structure: Is this right for you? → Critical warnings → Quick start → Step-by-step → Advanced patterns 4. Complete Quick Start Example ✅Lines 84-152 provide a fully runnable example upfront, much better than the previous scattered approach. 5. Visual Data Flow Diagram ✅Lines 162-206 show the workflow with ASCII art. Critical Issues1. Streaming Example Bug 🐛 (Lines 808-815)Issue: The code streams ALL content including the citation block, contradicting the critical warning. // Current code - WRONG
process.stdout.write(content); // Streams <<<CITATION_DATA>>> to users!Fix: Buffer everything, then extract visible text after streaming completes: // Don't stream raw content - buffer it
fullResponse += content;
// After streaming completes:
const visibleText = extractVisibleText(fullResponse);
// Now send visibleText to client2. Model Name Verification
|
PR Review: AGENTS.md RestructureThank you for this comprehensive rewrite of AGENTS.md! The restructuring significantly improves the document's usefulness for AI agents. Here's my detailed review: Strengths ✅1. Excellent Progressive Disclosure
2. Prevention of Agent Fabrication
3. Improved Technical Completeness
4. Better Organization
Issues & Recommendations 🔍High Priority1. Streaming Example Needs Clarification (AGENTS.md:846-873)The streaming example says "Don't stream raw content directly to users!" but doesn't clearly explain when to use Option A vs Option B: // IMPORTANT: Don't stream raw content directly to users!
// The <<<CITATION_DATA>>> block arrives at the end and must be stripped.
//
// Option A: Buffer everything, send after complete (simpler)
// Option B: Stream chunks but filter out the citation block (complex)Recommendation: Add a decision tree or more explicit guidance: // RECOMMENDED: Option A for most use cases
// Buffer the full response server-side, then extract visible text
// ADVANCED: Option B only if you need progressive streaming
// Requires detecting "<<<CITATION_DATA>>>" and stopping stream2. Model Name Comment Ambiguity (AGENTS.md:121, 836)The comment "Use your preferred model" appears multiple times but might confuse agents: model: "gpt-5-mini", // Use your preferred modelRecommendation: Be more explicit: model: "gpt-5-mini", // Replace with your model (e.g., "gpt-4o", "gpt-4-turbo")3. Missing File Hash Implementation (AGENTS.md:1081)The caching example uses async function getAttachmentId(fileHash: string, file: Buffer, filename: string) {
const cached = attachmentCache.get(fileHash);Recommendation: Add hash generation example: import { createHash } from 'crypto';
function getFileHash(file: Buffer): string {
return createHash('sha256').update(file).digest('hex');
}
async function getAttachmentId(file: Buffer, filename: string) {
const fileHash = getFileHash(file);
// ... rest of implementation
}Medium Priority4. Status Table Inconsistency (AGENTS.md:904-913)The "Detailed Status Values" table adds "UI Guidance" column, which is helpful, but some entries are vague:
Recommendation: Make UI guidance more actionable: 5. Error Handling Could Be More Robust (AGENTS.md:504-521)The error handling example is good but doesn't cover rate limiting (429 errors): Recommendation: Add rate limiting case: } catch (error: any) {
const message = error?.message || "Unknown error";
if (message.includes("rate limit") || error.status === 429) {
// Rate limit exceeded - implement exponential backoff
console.error("Rate limit exceeded. Retry after delay.");
} else if (message.includes("Invalid or expired API key")) {
// ...
}
}6. SSRF Warning Placement (AGENTS.md:279-283)The SSRF warning appears in a quote block after the URL example. This critical security concern could be more prominent. Recommendation: Move it above the code example or use a dedicated warning section: ### ⚠️ Security: URL Validation
When accepting user-provided URLs, **always validate** to prevent SSRF attacks:
- Block internal IPs: `localhost`, `127.0.0.1`, `192.168.*`, etc.
- Validate URL scheme is `http` or `https` only
```typescript
// Example validation...Recommendation: Show how to actually use this mapping: // Verify citation against specific document
const reportAttachmentId = fileDataParts[0].attachmentId;
await deepcitation.verifyAttachment(reportAttachmentId, citations);8. Verification Status Flags Table (AGENTS.md:928-939)Added Recommendation: Add a usage example in the "Check verification status" section (AGENTS.md:502). 9. Collapsible Sections AccessibilityUsing Recommendation: Consider using headers instead, or add a note at the top: <details>
<summary>Other LLM Providers (Claude, Gemini, Vercel AI SDK)</summary>
**Note**: These examples follow the same pattern as OpenAI - replace the client and model.Code QualityPositive
Suggestions
Security ConsiderationsWell Handled ✅
Could Improve
Test CoverageThe PR doesn't include tests for the documentation itself. Consider:
Performance ConsiderationsGood
Could Add
Overall AssessmentThis is a substantial improvement over the previous version. The document is now:
Recommendation: Approve with minor revisions (address High Priority issues) Suggested Next Steps
Great work addressing all 33 feedback items! This will significantly improve agent implementations. 🎉 |
- Fix streaming example: clarify that raw content should NOT be streamed to users; add comments explaining buffering strategy - Add model name clarification comment (use your preferred model) - Add Rate Limiting & Caching section with attachmentId caching example - Add input validation example in verify API route - Add SSRF security warning for user-provided URLs - Update production checklist with validation and caching items Co-Authored-By: Claude <noreply@anthropic.com>
2024790 to
65308bc
Compare
|
Comprehensive PR Review for AGENTS.md Restructuring OVERVIEW: STRENGTHS:
CRITICAL ISSUES (Must Fix):
MEDIUM PRIORITY:
LOW PRIORITY:
TEST COVERAGE:
OVERALL: Approve with Minor Improvements Files Changed: 1 file, +649/-295 lines Great work on this comprehensive rewrite! |
Summary
Complete rewrite of AGENTS.md to address 33 feedback items, making it more effective for AI code agents implementing DeepCitation.
Key Changes
Structure & Progressive Disclosure
Preventing Agent Fabrication
Technical Completeness
prepareUrl()method documentation (was missing)unsafeFastUrlOutputsecurity warning for untrusted URLsfileDataPartsmappingImproved Troubleshooting
Collapsible Sections
<details>for LLM provider alternatives (Anthropic, Gemini, Vercel AI SDK)<details>for React component variants reference<details>for unsafe URL mode documentationReference Improvements
Feedback Items Addressed
All 33 feedback items from the review have been addressed:
Test Plan
<details>sections render correctly in GitHub markdown