Skip to content

build(evaluation): add eval framework as a private submodule - #5190

Open
lenafiedor wants to merge 3 commits into
v3from
eval-submodule
Open

build(evaluation): add eval framework as a private submodule#5190
lenafiedor wants to merge 3 commits into
v3from
eval-submodule

Conversation

@lenafiedor

@lenafiedor lenafiedor commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Description

Added the evaluation module from ai-infrastructure as submodule for Klicker for the future chatbot evaluation and quality assessment.

ClickUp Task: Chatbot: Adapt evaluation module for Klicker

Proposed Changes

  • Add evaluation as submodule in Klicker
  • Add ground truth QA pairs at data/ground_truth/klicker_fineco

Automated Verification

  • Run pnpm run check:all (linting, typechecking, formatting, syncpack)
  • Run target unit tests (pnpm --filter <package> test)

Summary by CodeRabbit

  • New Features

    • Added evaluation materials covering finance concepts, portfolio theory, alpha, and covariance matrices.
    • Added synthetic ground-truth datasets with explanations, formulas, examples, and source references.
    • Added an optional evaluation framework that can be initialized explicitly when needed.
  • Documentation

    • Documented the evaluation assets, dataset contents, and framework setup instructions.

@lenafiedor
lenafiedor requested a review from rschlaefli as a code owner July 21, 2026 09:08
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. dependencies feature labels Jul 21, 2026
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

Safe to merge — the submodule is entirely opt-in (update = none, HTTPS URL), CI and regular clones are unaffected, and the committed data files contain only synthetic educational content with no personal data.

The change is purely additive: a new top-level directory with a passively registered submodule and committed Markdown ground-truth files. Nothing in the build, dev stack, or CI pipeline depends on the submodule being present.

Files Needing Attention: No files require special attention. The evaluation/README.md could benefit from a companion wiki entry under docs/, but the README itself is accurate and complete.

Important Files Changed

Filename Overview
.gitmodules Registers evaluation/framework as a submodule over HTTPS with update = none; correctly configured so CI and recursive clones skip it
evaluation/README.md Documents the submodule opt-in pattern and data directory contents clearly; the initialization command is correct
evaluation/framework Submodule entry pinned to commit 8ddc718; update = none means it is not fetched during regular workflow
evaluation/data/ground_truth/klicker_fineco/gt_001_cml_sharpe_ratio.md Synthetic CAPM/CML ground-truth QA; technically correct financial content, no personal data
evaluation/data/ground_truth/klicker_fineco/gt_008_covariance_matrix_conditions.md Synthetic covariance matrix QA; determinant check for matrix D is correct (det = 5x1 minus 9 = -4 < 0)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[git clone uzh-bf/klicker-uzh] --> B{--recurse-submodules?}
    B -- yes --> C[evaluation/framework skipped\nupdate = none]
    B -- no --> C
    C --> D[evaluation/data/ committed normally\nground-truth .md files available]
    E[UZH-internal user] --> F[git submodule update\n--init --checkout\nevaluation/framework]
    F --> G[evaluation/framework\nmaterialized from\ngitlab.uzh.ch/ai-infrastructure/evaluation]
    G --> H[DeepEval harness\nreads data/ground_truth/]
    D --> H
    H --> I[Chatbot quality\nassessment results]
Loading

Reviews (4): Last reviewed commit: "fix(evaluation): restructure eval submod..." | Re-trigger Greptile

Comment thread .gitmodules Outdated
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The evaluation submodule is registered and updated, its usage is documented, and eight financial economics ground-truth Markdown entries are added covering portfolio theory, alpha, and covariance-matrix topics.

Changes

Evaluation module integration

Layer / File(s) Summary
Submodule registration and documentation
.gitmodules, evaluation/README.md, evaluation/framework
Registers the private evaluation framework submodule with updates disabled, records its new commit, and documents the framework and synthetic datasets.

Portfolio and CAPM ground truth

Layer / File(s) Summary
Portfolio and CAPM entries
evaluation/data/ground_truth/klicker_fineco/gt_001_*.md, gt_002_*.md, gt_006_*.md
Adds ground-truth explanations for the CML Sharpe-ratio slope, tangency-portfolio risk-aversion allocation, and portfolio-weight interpretation.

Alpha ground truth

Layer / File(s) Summary
Alpha entries
evaluation/data/ground_truth/klicker_fineco/gt_003_*.md, gt_004_*.md, gt_005_*.md, gt_007_*.md
Adds explanations for weighted zero-sum alpha, heterogeneous ex-ante alpha, ex-post alpha measurement, and deriving a third investor’s alpha.

Covariance ground truth

Layer / File(s) Summary
Covariance-matrix entry
evaluation/data/ground_truth/klicker_fineco/gt_008_covariance_matrix_conditions.md
Defines covariance-matrix validity criteria and evaluates four candidate matrices, identifying matrix C as valid.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: rschlaefli

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding the evaluation framework as a private submodule.
Description check ✅ Passed The description covers the summary, ClickUp task, proposed changes, and automated verification, with only some template sections left incomplete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rschlaefli rschlaefli changed the title feat(modules/eval): Eval as a submodule build(evaluation): add eval framework as a private submodule Jul 27, 2026
lenafiedor and others added 3 commits July 27, 2026 10:13
…ones

- move submodule modules/eval -> evaluation/framework
- use HTTPS URL and update=none so clones (incl. --recursive) never fail
  on the UZH-private framework repo; internal opt-in init documented
- move ground-truth data under evaluation/data/
- add evaluation/README.md describing the layout

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@evaluation/data/ground_truth/klicker_fineco/gt_007_third_investor_alpha.md`:
- Around line 9-21: Define r^i, r^j, and r^3 consistently as wealth weights,
clarify whether they are raw wealth or normalized shares, and use that notation
throughout the derivation. Explicitly state that investor 3 has positive (and
therefore nonzero) wealth before dividing by r^3.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 82e88366-cc3e-4c76-837c-7b4ebae75ae6

📥 Commits

Reviewing files that changed from the base of the PR and between c8de9c8 and 8a27d23.

📒 Files selected for processing (11)
  • .gitmodules
  • evaluation/README.md
  • evaluation/data/ground_truth/klicker_fineco/gt_001_cml_sharpe_ratio.md
  • evaluation/data/ground_truth/klicker_fineco/gt_002_tangency_portfolio_risk_aversion.md
  • evaluation/data/ground_truth/klicker_fineco/gt_003_weighted_alpha_zero_sum.md
  • evaluation/data/ground_truth/klicker_fineco/gt_004_ex_ante_alpha_heterogeneous_expectations.md
  • evaluation/data/ground_truth/klicker_fineco/gt_005_ex_post_alpha_same_measure.md
  • evaluation/data/ground_truth/klicker_fineco/gt_006_lambda_portfolio_weights.md
  • evaluation/data/ground_truth/klicker_fineco/gt_007_third_investor_alpha.md
  • evaluation/data/ground_truth/klicker_fineco/gt_008_covariance_matrix_conditions.md
  • evaluation/framework

Comment on lines +9 to +21
Use the zero-sum property of alpha with wealth weights:

$$
\alpha^i r^i + \alpha^j r^j + \alpha^3 r^3 = 0
$$

Solve for the unknown alpha of investor 3:

$$
\alpha^3 = -\frac{\alpha^i r^i + \alpha^j r^j}{r^3}
$$

So you multiply each known investor's alpha by their wealth, add those terms, change the sign, and divide by the third investor's wealth.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Define the wealth weights and denominator assumption.

This entry switches from a^i in gt_003_weighted_alpha_zero_sum.md to r^i without defining whether r^i is raw wealth or a normalized wealth share. It also divides by r^3 without stating that the third investor has nonzero wealth. Use one notation consistently and state the positive-wealth assumption.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@evaluation/data/ground_truth/klicker_fineco/gt_007_third_investor_alpha.md`
around lines 9 - 21, Define r^i, r^j, and r^3 consistently as wealth weights,
clarify whether they are raw wealth or normalized shares, and use that notation
throughout the derivation. Explicitly state that investor 3 has positive (and
therefore nonzero) wealth before dividing by r^3.

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

dependencies feature size:L This PR changes 100-499 lines, ignoring generated files.

Development

Successfully merging this pull request may close these issues.

2 participants