Skip to content

refactor(core): centralize prompt discovery in PromptLoader and cleanup AssertionEngine#44

Open
silkiy wants to merge 3 commits into
Evaliphy:mainfrom
silkiy:main
Open

refactor(core): centralize prompt discovery in PromptLoader and cleanup AssertionEngine#44
silkiy wants to merge 3 commits into
Evaliphy:mainfrom
silkiy:main

Conversation

@silkiy

@silkiy silkiy commented Apr 20, 2026

Copy link
Copy Markdown

Description

This PR addresses structural issues and dead code identified in AssertionEngine.ts (Issue #43). The logic for resolving prompt file paths has been extracted from the engine and centralized into a dedicated service within PromptLoader.

Key Changes

  • Cleaned up AssertionEngine: Removed the unused searchPaths variable and deleted the legacy loadPrompt method. The engine now delegates path resolution to PromptLoader.
  • Implemented PromptLoader.resolveAndLoad: Centralized the discovery logic with the following priority:
    1. User Config: Respects promptsDir set in evaliphy.config.ts.
    2. SDK Dist: Fallback to bundled prompts in ./prompts.
    3. SDK Source: Fallback to source prompts in ../../prompts (useful for development).
  • Robust Error Handling: Improved the FILE_NOT_FOUND and PROMPT_LOAD_ERROR handling to provide clearer debugging information to users.
  • Comprehensive Unit Tests: Added promptLoader.test.ts covering all resolution scenarios, fallback mechanisms, and frontmatter validation using vitest.

Validation

  • Ran pnpm exec tsc --noEmit (Type-check passed).
  • Ran pnpm exec vitest run packages/assertions/tests/judge/promptLoader.test.ts (All tests passed).

Closes #43

@vercel

vercel Bot commented Apr 20, 2026

Copy link
Copy Markdown

@silkiy is attempting to deploy a commit to the priyanshus's projects Team on Vercel.

A member of the Team first needs to authorize it.

/**
* Service to handle centralized prompt searching and loading.
*/
export class PromptLoader {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we break this function further?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I agree the previous function was doing too much. I've broken it down into smaller, single-responsibility private methods (checkUserConfig, checkSdkDist, and checkSdkSource) so resolvePromptPath just acts as a clean orchestrator now. Just pushed the update!

@silkiy silkiy requested a review from priyanshus April 20, 2026 15:58
@priyanshus

Copy link
Copy Markdown
Collaborator

@silkiy Please check the tests. A few failed. Thanks.

@priyanshus priyanshus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Core] Refactor AssertionEngine to address issues from description

2 participants