General: Display total LLM cost in course detail view
#10019
Conversation
Introduces the calculation and visualization of total LLM costs per course. Back-end changes include a repository query and DTO update, while front-end changes add a doughnut chart and i18n labels to display the cost. This enhances transparency of LLM usage.
wasnertobias
added
feature
ready for review
server
github-actions
bot
added
the
core
wasnertobias
changed the title
Development: Display total LLM cost in course detail view
WalkthroughThis pull request introduces functionality to track and display the total LLM (Large Language Model) cost for a course. The changes span multiple files across the backend and frontend, adding a new field Changes
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseManagementDetailViewDTO.java (1)
11-11: Consider using primitive double for cost field.The implementation looks good, but since LLM cost should never be null (defaulting to 0.0), consider using primitive
doubleinstead ofDoubleto improve memory efficiency and prevent potential NPEs.src/main/webapp/app/course/manage/course-management-detail-view-dto.model.ts (1)
29-31: LGTM! Consider future LLM metrics.Good organization with the "LLM Stats" section comment. This structure will make it easier to add more LLM-related metrics in the future if needed.
src/main/java/de/tum/cit/aet/artemis/core/repository/LLMTokenUsageTraceRepository.java (1)
18-19: Consider using BigDecimal for financial calculations.The current implementation using floating-point arithmetic might lead to precision issues. Consider moving the calculation to the service layer using
BigDecimalfor more accurate financial computations.src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.ts (1)
Line range hint
53-59: Consider empty string edge case in ngOnChangesThe condition
!this.showTextwill evaluate to true for empty strings, which might lead to unintended fallback behavior.Consider using strict undefined check:
-if (this.currentAbsolute == undefined && !this.receivedStats && !this.showText) { +if (this.currentAbsolute == undefined && !this.receivedStats && this.showText === undefined) {src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
233-233: Consider breaking down the service dependenciesThe constructor has a large number of dependencies (20+ parameters). Consider breaking this service into smaller, more focused services to improve maintainability and follow the Single Responsibility Principle.
Consider creating separate services for specific functionality domains, for example:
- CourseEnrollmentService
- CourseStatisticsService
- CourseArchiveService
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseManagementDetailViewDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/repository/LLMTokenUsageTraceRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java(6 hunks)src/main/webapp/app/course/manage/course-management-detail-view-dto.model.ts(1 hunks)src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.html(1 hunks)src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.ts(3 hunks)src/main/webapp/app/course/manage/detail/course-detail.component.html(1 hunks)src/main/webapp/app/course/manage/detail/course-detail.component.ts(1 hunks)src/main/webapp/i18n/de/course.json(1 hunks)src/main/webapp/i18n/en/course.json(1 hunks)src/test/javascript/spec/component/course/detail/course-detail-doughnut-chart.component.spec.ts(1 hunks)src/test/javascript/spec/component/course/detail/course-detail.component.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/webapp/app/course/manage/detail/course-detail.component.ts
- src/main/webapp/i18n/de/course.json
🧰 Additional context used
📓 Path-based instructions (9)
src/main/webapp/app/course/manage/course-management-detail-view-dto.model.ts (1)
src/test/javascript/spec/component/course/detail/course-detail.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/course/detail/course-detail-doughnut-chart.component.spec.ts (1)
Pattern src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/main/webapp/app/course/manage/detail/course-detail.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/java/de/tum/cit/aet/artemis/core/repository/LLMTokenUsageTraceRepository.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.html (1)
Pattern src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.ts (1)
src/main/java/de/tum/cit/aet/artemis/core/dto/CourseManagementDetailViewDTO.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)
Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (10)
src/main/java/de/tum/cit/aet/artemis/core/repository/LLMTokenUsageTraceRepository.java (1)
6-7: LGTM! Clean import organization.
The imports are well-organized and specific (no wildcard imports).
src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.html (2)
12-15: LGTM! Proper usage of new Angular syntax.
The change from *ngIf to @if follows the new Angular syntax guidelines correctly.
12-20: Consider handling edge cases in the conditional logic.
The current implementation might have edge cases:
- When
currentPercentageis 0, it might be treated as undefined - When
showTextis an empty string, it might lead to undesired UI state
Consider adding explicit null checks and handling empty strings.
- @if (receivedStats && currentPercentage != undefined) {
+ @if (receivedStats && currentPercentage !== undefined && currentPercentage !== null) {
<div class="h4">{{ currentPercentage }}%</div>
} @else if (receivedStats && showText) {
- <div class="h4">{{ showText }}</div>
+ <div class="h4">{{ showText || '' }}</div>
}src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.ts (2)
24-24: LGTM: Input property correctly typed
The new input property showText is properly typed as string | undefined.
88-91: LGTM: LLM cost chart configuration
The new chart type is properly configured with appropriate title and undefined link.
src/main/webapp/i18n/en/course.json (1)
262-263: LGTM: Translation key properly added
The new translation key follows the existing naming convention and provides a clear, concise description.
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (4)
86-86: LGTM!
The import statement follows Java conventions and is properly organized within the core repository imports section.
218-219: LGTM!
The field declaration follows Spring best practices with proper encapsulation and constructor injection pattern.
277-277: LGTM!
The field initialization follows the established pattern and is properly placed within the constructor.
861-862: Verify LLM cost calculation implementation
The LLM cost calculation is delegated to the repository layer. Let's verify the implementation of calculateTotalLlmCostInEurForCourse to ensure proper cost calculation.
✅ Verification successful
LLM cost calculation implementation is properly implemented
The repository method calculateTotalLlmCostInEurForCourse is correctly implemented with:
- A proper SQL query that calculates costs based on both input and output tokens
- Cost calculation using token counts and their respective costs per million tokens
- Proper rounding to 2 decimal places
- Null-safe implementation returning Double type
- Integration with the course management view and UI components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the implementation of LLM cost calculation in the repository
# Expected: Find the repository method implementation and any related tests
# Search for the repository method implementation
echo "Searching for repository method implementation..."
ast-grep --pattern 'calculateTotalLlmCostInEurForCourse($id) {
$$$
}'
# Search for related test cases
echo "Searching for related test cases..."
rg -l "calculateTotalLlmCostInEurForCourse.*Test"
Length of output: 315
Script:
#!/bin/bash
# Let's try a different approach to find the repository implementation
# First, let's find the repository interface or class
echo "Searching for LlmTokenUsageTraceRepository..."
rg -l "LlmTokenUsageTraceRepository"
# Search for any method containing calculateTotalLlmCostInEur
echo "Searching for cost calculation method..."
rg "calculateTotalLlmCostInEur" -A 5
# Look for related test files
echo "Searching for related test files..."
fd -e java -e kt test.*LlmTokenUsageTrace
Length of output: 1769
Script:
#!/bin/bash
# Let's check the repository implementation details
# Look for SQL query or implementation details
echo "Searching for SQL query or implementation..."
rg -B 5 -A 1 "calculateTotalLlmCostInEurForCourse" --type java
# Check if there's any @Query annotation associated
echo "Searching for @Query annotation..."
rg -B 3 "calculateTotalLlmCostInEurForCourse" --type java
# Look for any cost-related constants or configurations
echo "Searching for cost-related configurations..."
rg "llm.*cost|cost.*llm" -i
Length of output: 8018
src/main/java/de/tum/cit/aet/artemis/core/repository/LLMTokenUsageTraceRepository.java
Show resolved
Hide resolved
src/main/webapp/app/course/manage/detail/course-detail.component.html
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/course/detail/course-detail-doughnut-chart.component.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/component/course/detail/course-detail.component.spec.ts
Show resolved
Hide resolved
HawKhiem
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
GitHub Actions
Inactive
src/main/webapp/app/course/manage/detail/course-detail.component.html
Outdated
Show resolved
Hide resolved
|
src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.html
Outdated
Show resolved
Hide resolved
src/main/webapp/app/course/manage/detail/course-detail-doughnut-chart.component.ts
Outdated
Show resolved
Hide resolved
wasnertobias
dismissed stale reviews from HawKhiem, sachmii, and Malekos74
via
4bc3d38
github-actions
bot
added
the
database
sawys777
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
|
github-actions
bot
added
the
deployment-error
krusche
changed the title
Development: Display total LLM cost in course detail viewGenera: Display total LLM cost in course detail view
krusche
changed the title
Genera: Display total LLM cost in course detail viewGeneral: Display total LLM cost in course detail view
Checklist
General
Server
Client
multiplescreenshots/screencasts of my UI changes.Motivation and Context
LLM usage of IRIS and Athena has been tracked for a few weeks but has not yet been shown anywhere to users. This should change, and as a first approach, the total LLM cost of a course should be shown in the course details. In a later approach (TBD), the design will be improved to display usage on specific days for a given course.
Description
Introduces the calculation and visualization of total LLM costs per course in course details view. Back-end changes include a repository query and DTO update, while front-end changes add a doughnut chart and i18n labels to display the cost. This enhances transparency of LLM usage.
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Localization
Tests