refactor: replace analysis field with issue/message pair#53
refactor: replace analysis field with issue/message pair#53
Conversation
- Rename `analysis` field to `issue` and add `message` field - Add `--suggest` CLI flag to cli-schemas - Update subjective and semi-objective LLM schemas - Add prompt guidance for scope-aware message formatting - Update scoring logic to use new fields for deduplication - Add rule for naming sections in comparisons
📝 WalkthroughWalkthroughRefactors violation data to replace Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Orchestrator
participant Evaluator
participant Reporter
User->>CLI: run command (with or without --suggest)
CLI->>Orchestrator: evaluateFile(params..., suggest)
Orchestrator->>Evaluator: process file -> produce violations (issue, message, suggestion)
Evaluator-->>Orchestrator: return results + token usage
Orchestrator->>Reporter: route results + suggest flag
Reporter-->>User: emit line output (include suggestions only if suggest true)
Reporter-->>User: emit JSON output (include suggestions)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)src/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/prompts/directive-loader.ts:
- Around line 33-44: The documentation has inconsistent thresholds: the line
containing "SENTENCE-level (quoted_text is >5 words)" conflicts with the
CRITICAL rule "If quoted_text is longer than 4 words"; update the SENTENCE-level
wording to "SENTENCE-level (quoted_text is >4 words)" so it matches the CRITICAL
rule and any other references; locate the two strings "SENTENCE-level
(quoted_text is >5 words)" and "If quoted_text is longer than 4 words" in
directive-loader.ts and make the first one use ">4" to ensure the threshold is
consistent.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/chunking/merger.tssrc/cli/commands.tssrc/cli/orchestrator.tssrc/cli/types.tssrc/prompts/directive-loader.tssrc/prompts/schema.tssrc/schemas/cli-schemas.tssrc/scoring/scorer.tstests/scoring-types.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use TypeScript ESM with explicit imports and narrow types
Use 2-space indentation; avoid trailing whitespace
Maintain strict TypeScript with noany; useunknown+ schema validation for external data
Use custom error types with proper inheritance; catch blocks useunknowntype
Files:
src/chunking/merger.tssrc/cli/commands.tssrc/prompts/schema.tssrc/scoring/scorer.tssrc/schemas/cli-schemas.tssrc/cli/types.tssrc/prompts/directive-loader.tssrc/cli/orchestrator.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.test.ts: Write tests using Vitest framework with focus on config parsing, file discovery, schema/structured output, and locator
Use dependency injection in tests: mock providers; do not hit network in unit tests
Files:
tests/scoring-types.test.ts
🧠 Learnings (8)
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Applies to src/output/**/*.ts : IDs should be shown as `PromptId.CriterionId` in output
Applied to files:
tests/scoring-types.test.tssrc/cli/types.tssrc/cli/orchestrator.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Applies to tests/**/*.test.ts : Write tests using Vitest framework with focus on config parsing, file discovery, schema/structured output, and locator
Applied to files:
tests/scoring-types.test.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Keep directories cohesive; maintain acyclic dependencies (CLI → prompts/providers/output); optimize for discoverability
Applied to files:
src/cli/commands.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Applies to src/providers/**/*.ts : Depend on `LLMProvider` and `SearchProvider` interfaces; keep providers thin (transport only)
Applied to files:
src/cli/commands.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Applies to src/boundaries/env-parser.ts : All environment variables must be validated via Zod schemas in `src/boundaries/env-parser.ts`
Applied to files:
src/cli/commands.tssrc/schemas/cli-schemas.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Recursively load all `.md` files from within each rule pack; multiple packs can be used simultaneously
Applied to files:
src/cli/commands.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Organize rules into subdirectories (packs) within RulesPath; use company names (e.g., Acme, TechCorp, Stripe) to indicate which style guide the rules implement
Applied to files:
src/cli/commands.ts
📚 Learning: 2025-12-28T19:43:51.189Z
Learnt from: CR
Repo: TRocket-Labs/vectorlint PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T19:43:51.189Z
Learning: Applies to src/boundaries/**/*.ts : Use Zod schemas for boundary validation of all external data (files, CLI, env, APIs) at system boundaries
Applied to files:
src/schemas/cli-schemas.ts
🧬 Code graph analysis (2)
src/cli/commands.ts (5)
src/config/constants.ts (1)
DEFAULT_CONFIG_FILENAME(12-12)src/errors/index.ts (1)
handleUnknownError(46-51)src/config/preset-loader.ts (1)
PresetLoader(5-52)src/boundaries/rule-pack-loader.ts (1)
RulePackLoader(12-95)src/providers/perplexity-provider.ts (1)
PerplexitySearchProvider(11-54)
src/cli/orchestrator.ts (1)
src/providers/token-usage.ts (1)
calculateCost(21-30)
🔇 Additional comments (23)
src/chunking/merger.ts (1)
8-14: LGTM!The deduplication key update correctly replaces
analysiswithmessage, maintaining consistency with the new violation schema. The composite key construction and normalization logic remain sound.src/scoring/scorer.ts (4)
56-64: LGTM!The violation mapping correctly adopts the new
issueandmessagefields while preserving the optional field handling pattern.
178-185: LGTM!The type definition for aggregated violations correctly includes the new
issueandmessagefields.
214-224: LGTM!Violation collection correctly extracts
issueandmessagewith appropriate fallbacks for missing values.
254-264: LGTM!The deduplication key correctly uses
messageinstead ofanalysis. The two-component key (quoted_text + message) is appropriate here since violations are already grouped by criterion name withincriteriaMap.tests/scoring-types.test.ts (3)
32-32: LGTM!The
packproperty addition aligns with the updatedPromptFiletype.
100-118: LGTM!The mock violation data correctly includes both
issueandmessagefields, reflecting the updated schema structure. The test assertions remain valid for verifying scoring behavior.
188-188: LGTM!Consistent
packproperty addition for technical accuracy test data.src/schemas/cli-schemas.ts (1)
8-9: LGTM!The
suggestoption follows the established pattern for boolean CLI options with afalsedefault. The Zod schema provides proper boundary validation as per coding guidelines.src/cli/commands.ts (3)
40-40: LGTM!The
--suggestoption is correctly registered with Commander, following the established pattern for boolean flags.
208-223: LGTM!The
suggestparameter is correctly threaded through to theevaluateFilesorchestrator call, enabling conditional suggestion output based on the CLI flag.
100-103: LGTM!Useful verbose logging addition for debugging directive-related issues.
src/prompts/directive-loader.ts (1)
24-28: Clear separation of issue and message with scope-aware guidance.The new structure clearly defines:
issue: Problem description without quoted_text (5-12 words)message: User-facing output with scope-dependent formattingThis aligns well with the PR objective of enabling scope-aware message formatting.
src/cli/types.ts (2)
37-37: Newsuggestflag properly propagated through options.The optional
suggestboolean is correctly added toEvaluationOptions, enabling CLI control over suggestion visibility.
81-94: Violation structure updated to useissue/messagepair.The
ProcessViolationsParams.violationstype now includesissue?: stringandmessage?: string, replacing the previousanalysisfield. All fields remain optional to handle partial data gracefully.src/prompts/schema.ts (3)
36-46: Subjective schema correctly updated with requiredissueandmessagefields.The schema now requires both
issueandmessagefor violations, replacing the previousanalysisfield. The TypeScript typeSubjectiveLLMResult(lines 110-111) aligns with this schema.
79-91: Semi-objective schema updated consistently.Both
issueandmessageare added to the schema and marked as required (lines 88-89), with corresponding type updates inSemiObjectiveLLMResult(lines 120-121).
152-160:SemiObjectiveItemtype correctly reflects the new structure.The type replaces
analysiswithissueandmessagefields, maintaining consistency with the schema definition.src/cli/orchestrator.ts (5)
245-248: Suggestion visibility logic is well-documented and correct.The
includeSuggestionlogic properly gates suggestions:
- Line output: Only when
--suggestflag is used- JSON formats: Always included for machine consumption
The inline comments clearly explain this behavior.
429-442: Violation type and suggest flag correctly propagated in subjective path.The type cast at lines 424-432 properly includes
issue?andmessage?fields. Thesuggestflag is coerced with!!for consistent boolean handling.
599-609: Improved type annotation for violation grouping.The explicit generic type
Map<string | undefined, typeof result.violations>enhances type safety for the criterion-grouped violations map.
1019-1025: Formatting improvement for readability.The multi-line formatting of the
calculateCostcall improves readability without changing behavior.
193-196: The violation schema definesmessageandissueas distinct fields with separate purposes. Both are required in the LLM response schema and handled independently throughout the codebase (see src/prompts/schema.ts and scorer.ts). Usingmessagefor the displayed summary is the intended design. The defensive fallback to an empty string is safe and prevents crashes; there is no evidence thatissueshould be used as a fallback formessage.Likely an incorrect or invalid review comment.
…o refactor/output-format
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/prompts/schema.ts (1)
117-127: Schema-type mismatch:suggestionis required in schema but optional in type.The
buildSemiObjectiveLLMSchemamarkssuggestionas required (line 90), butSemiObjectiveLLMResultdeclares it as optional (suggestion?: string). This inconsistency could cause runtime validation failures when the LLM omits the suggestion field.Either make
suggestionrequired in the type or remove it from therequiredarray in the schema for consistency.Proposed fix (Option A - make type required)
export type SemiObjectiveLLMResult = { violations: Array<{ description: string; issue: string; message: string; - suggestion?: string; + suggestion: string; quoted_text?: string; context_before?: string; context_after?: string; }>; };Proposed fix (Option B - make schema optional)
required: [ "quoted_text", "context_before", "context_after", "description", "issue", "message", - "suggestion", ],
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cli/commands.tssrc/prompts/schema.tssrc/scoring/scorer.tstests/scoring-types.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/scoring-types.test.ts
- src/cli/commands.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use TypeScript ESM with explicit imports and narrow types
Use 2-space indentation; avoid trailing whitespace
Maintain strict TypeScript with noany; useunknown+ schema validation for external data
Use custom error types with proper inheritance; catch blocks useunknowntype
Files:
src/scoring/scorer.tssrc/prompts/schema.ts
🔇 Additional comments (8)
src/prompts/schema.ts (4)
36-46: LGTM on the issue/message field additions.The schema correctly defines both
issueandmessageas required properties with proper string types. The structure aligns with the PR objectives for scope-aware message formatting.
79-91: LGTM on the semi-objective schema changes.The
issueandmessagefields are properly added to both the properties and required arrays, maintaining consistency with the subjective schema structure.
152-160: LGTM on the SemiObjectiveItem type update.The type correctly reflects the new
issueandmessagefields while maintaining optional fields forsuggestion,quoted_text, and context fields.
170-178: LGTM on the SemiObjectiveResult violations type.The violations array properly includes the new
issueandmessagefields as required, with appropriate optional fields.src/scoring/scorer.ts (4)
56-64: LGTM on the violation mapping.The mapping correctly extracts
issueandmessagefromSemiObjectiveItemand properly uses conditional spreads for optional fields.
171-188: LGTM on the type definition update.The violations array type correctly reflects the new
issueandmessagefields, maintaining consistency with the schema types defined inschema.ts.
214-222: LGTM on violation collection.The fallback to empty strings (
v.issue || "",v.message || "") provides safe defaults, consistent with how other fields are handled.
254-263: LGTM on the deduplication key update.Using
messagefor the deduplication key is a sensible choice since it represents the unique user-facing description of each violation. The composite key combiningquoted_textandmessageprovides good uniqueness guarantees.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/prompts/schema.ts (1)
117-127: Type/schema mismatch for optional fields.The
SemiObjectiveLLMResulttype marksquoted_text,context_before, andcontext_afteras optional (lines 123-125), but the JSON schema inbuildSemiObjectiveLLMSchema(lines 83-86) lists them as required. This inconsistency can cause type-safety issues when parsing LLM responses.Either update the type to make these fields required, or remove them from the schema's
requiredarray.Option 1: Make type fields required to match schema
export type SemiObjectiveLLMResult = { violations: Array<{ description: string; issue: string; message: string; suggestion: string; - quoted_text?: string; - context_before?: string; - context_after?: string; + quoted_text: string; + context_before: string; + context_after: string; }>; };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/prompts/directive-loader.tssrc/prompts/schema.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use TypeScript ESM with explicit imports and narrow types
Use 2-space indentation; avoid trailing whitespace
Maintain strict TypeScript with noany; useunknown+ schema validation for external data
Use custom error types with proper inheritance; catch blocks useunknowntype
Files:
src/prompts/directive-loader.tssrc/prompts/schema.ts
🔇 Additional comments (7)
src/prompts/schema.ts (3)
26-51: Schema changes look correct.The
issueandmessagefields are properly added to the violations schema with correct types. Therequiredarray is updated appropriately. Note thatlineremains optional (not in required), which appears intentional for cases where line numbers cannot be determined.
68-91: Semi-objective schema correctly updated.The schema properly adds
issueandmessageas required fields while retainingdescriptionfor this evaluation type. The field structure is consistent with the TypeScript type definition below.
152-180: Processed result types correctly updated.The
SemiObjectiveItemandSemiObjectiveResulttypes properly includeissueandmessagefields. The optional markers onsuggestion,quoted_text,context_before, andcontext_afterare appropriate for these processed/downstream types.src/prompts/directive-loader.ts (4)
24-28: Clear guidance for issue/message distinction.The directive clearly explains the new
issueandmessagefields with scope-based formatting rules. The examples demonstrate the expected format for WORD-level vs SENTENCE/SECTION/DOCUMENT-level messages.
31-44: Well-structured examples with clear scope distinctions.The MESSAGE EXAMPLES section provides concrete illustrations for each scope level (WORD, SENTENCE, SECTION). The critical rule about the 4-word threshold is prominently highlighted.
47-54: Critical rules properly updated.Rule 6 (line number matching) and Rule 7 (naming sections when comparing) are important additions that improve output quality and consistency with the new schema structure.
56-68: loadDirective function unchanged and correct.The function properly handles the precedence chain (project override → built-in) with appropriate error handling. The empty catch block is acceptable here since the intent is to gracefully fall back to the built-in directive.
1 participant
Summary
Refines the analysis output format by replacing the single
analysisfield with a structuredissue+messagepair. This enables scope-aware formatting, where the message can intelligently include or exclude the quoted text based on the issue scope (word, sentence, section, or document).Changes
Prompt Improvements (
src/prompts/directive.ts)quoted_textis longer than 4 words, do NOT include it in the messageSchema Updates (src/prompts/schema.ts)
analysiswithissueandmessagefields in both subjective and semi-objective schemasCLI (src/schemas/cli-schemas.ts)
suggestboolean option for showing suggestionsScoring (src/scoring/scorer.ts)
issue/messagefieldsmessageinstead ofanalysisTests
packproperty and new field structureSummary by CodeRabbit
New Features
Other Changes
✏️ Tip: You can customize this high-level summary in your review settings.