fix(prompt): default Java 25 environment stops unnecessary version questions; ESLint replaced by oxlint with Zod and naming rules#24
Conversation
…ersion The system prompt had 6 separate instructions telling the LLM to "ask for version/confirmation" before answering Java questions, but no instruction establishing a default environment. This made the assistant pedantically ask "what Java version are you on?" even when it had just retrieved and cited the correct JDK docs. The compounding effect of these rules made the chat experience frustrating for users. Add a Default Environment section that assumes Java 25 with preview disabled, and rewrite the version/quality rules across all prompt modes (core, low-quality search, guided learning, code review) to only ask when the user contradicts the default or the answer genuinely varies across versions. - Add Default Environment section with explicit "do NOT ask" constraint - Rewrite Data Sources rule to supplement with general knowledge instead of blocking on missing details - Add rule to state retrieved facts confidently without hedging - Rewrite Version Awareness to trust retrieved docs and skip version caveats for features finalized before the configured JDK version - Remove "ask for version/source" from low-quality search prompt - Remove "ask a clarifying question" from guided learning prompt - Remove "ask for the exact file or version" from code review prompt
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated system prompt defaults and guidance (assume Java version context, preview disabled by default), refined RAG/fallback and version-awareness rules, adjusted multiple prompt variants, and made small service-code cleanups (health null-check simplification and LinkedHashMap construction change). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c00e87d52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/main/java/com/williamcallahan/javachat/config/SystemPromptConfig.java
Outdated
Show resolved
Hide resolved
ESLint added ~150MB of dependencies (@typescript-eslint/parser, eslint core) for a single naming-convention rule. oxlint already handles correctness, suspicious, and type-aware linting natively. This replaces ESLint entirely with: - eslint-plugin-zod as an oxlint JS plugin for Zod-specific lint rules - Enhanced oxlintrc.json with 12 Zod rules (no-any-schema, require-error-message, etc.) - Rule overrides for legitimate patterns (side-effect imports, sequential stream awaits) - Test-file overrides for Zod suffix/message rules - Moved config from root oxlintrc.json to config/oxlintrc.json for organization - Remove eslint, @typescript-eslint/eslint-plugin, @typescript-eslint/parser - Add eslint-plugin-zod as devDependency - Replace lint:es script with lint:sg (ast-grep) stage - Update all oxlint -c paths to config/oxlintrc.json
ESLint's @typescript-eslint/naming-convention was the only rule justifying the ESLint dependency. These three ast-grep rules replace it with faster, more targeted checks aligned to CLAUDE.md naming discipline rules. - Ban generic type suffixes (*Data, *Info, *Object, etc.) per [ND1f] - Ban Hungarian "I" prefix on interfaces per [NM2] - Require PascalCase for type aliases, interfaces, and enums per [ND1f] - Add sgconfig.yml pointing to rules/ast-grep directory
Frontend linting was not enforced by `make lint` or the prek pre-push hook, allowing violations to accumulate. This adds a dedicated `lint-frontend` target and wires it into both the lint target and the pre-push hook so violations are caught before they reach the remote. - Add lint-frontend Makefile target running pnpm lint + svelte-check - Wire lint-frontend into the existing lint target - Add lint step to prek.toml pre-push hooks (runs before build+test)
The new oxlint config surfaced 6 violations that are all false positives on legitimate code patterns. Two require source-level fixes since oxlint overrides cannot disable category-enabled rules per-file. - Add oxlint-disable-next-line for jsdom scrollTo polyfill in test setup - Replace bare await with expect().resolves.toBeUndefined() in scroll smoke test
The eslint-plugin-zod rule zod/no-number-schema-with-int flags z.number().int() as redundant when z.int() exists as a first-class Zod v4 API. Verified against zod 3.25.76 source: z.int() returns ZodNumberFormat with format 'safeint', functionally identical to z.number().int(). - Replace z.number().int().positive() with z.int().positive() in SSE schemas
The Makefile has always used `npm` for frontend commands, but pnpm-lock.yaml was introduced by mistake. npm's strict peer dep resolution rejects eslint-plugin-zod because it declares peerOptional zod@"^4" while zod's v4 API ships under 3.25.x semver. The override tells npm to use the project's zod version for that plugin. - Delete pnpm-lock.yaml and regenerate package-lock.json via npm install - Replace pnpm references in lint/validate scripts with npm run - Add overrides entry to resolve eslint-plugin-zod peer dep on zod
Summary
The chat assistant pedantically asked "what Java version are you on?" on every question — even when it had just retrieved and cited the correct JDK 25 docs. The system prompt had six separate "ask for version/confirmation" instructions but no instruction establishing a default. This PR adds a default Java 25 environment assumption and rewrites all prompt modes to only ask when the user contradicts that default. Separately, the frontend lint pipeline drops ESLint (150 MB of dependencies for one naming-convention rule) in favor of oxlint with Zod-specific rules and ast-grep naming enforcement.
Changes
Bug Fixes
SystemPromptConfig.java)SystemPromptConfig.CORE_PROMPT_TEMPLATE)SystemPromptConfig.CORE_PROMPT_TEMPLATE)Refactoring
eslint,@typescript-eslint/eslint-plugin, and@typescript-eslint/parser(one naming rule was the only justification). oxlint now runs correctness, type-aware, and Zod-specific checks natively viaeslint-plugin-zodas a JS plugin with 12 Zod rules (frontend/config/oxlintrc.json,frontend/package.json)@typescript-eslint/naming-convention— ban generic type suffixes like*Data/*Infoper [ND1f], ban HungarianIprefix on interfaces, and require PascalCase on types/interfaces/enums (frontend/rules/ast-grep/)make lintnow includes alint-frontendtarget (oxlint + ast-grep + svelte-check), and a separate lint step runs before build+test in the prek pre-push hook (Makefile,config/prek.toml)pnpmbut the project usesnpm; corrected all script references and regeneratedpackage-lock.json(frontend/package.json)isHealthy()is now a pure query; retry triggering moved to explicittriggerRetryIfDue()call, improving Command-Query Separation (ExternalServiceHealth.java)LinkedHashMap.newLinkedHashMap(size)(Java 19+) computes internal capacity to hold the expected number of entries without rehashing, unlike the constructor which uses raw capacity (HybridSearchService.java:141)ArithmeticException _in backoff computation where the exception type is sufficient and the instance carries no diagnostic value (ExternalServiceHealth.java:417)Test Improvements
awaitreplaced withexpect().resolves.toBeUndefined()to actually verify the return value (scroll.test.ts)z.int()shorthand:z.number().int()replaced withz.int()per the newzod/no-number-schema-with-intlint rule (schemas.ts)Breaking Changes
None
Test Plan
./gradlew buildpasses (pre-push hook verified)./gradlew testpasses (242 tests)npm run lintpasses (oxlint + ast-grep + svelte-check)npm run build)