feat(workspace): connect any OpenAI-compatible model, and cap what AI spends - #1400
Merged
Conversation
Give operators and workspace admins visibility into LLM spend, and stop a single workspace from quietly burning the whole instance budget. Consolidate every source of LLM cost — practice detection, replay, and mentor turns (web + Slack) — into one append-only ledger (`llm_usage_event`) instead of reading cost back out of `chat_message.metadata` JSON. The ledger is the accounting source of truth; the existing per-job columns and message metadata stay as diagnostics and the streaming wire contract. On top of the ledger: - Monthly budget cap per workspace (`workspace.monthly_llm_budget_usd`, instance-admin-set, null = uncapped, 0 = paused). When the current UTC month's spend reaches the cap, new detection jobs and mentor turns pause until the month rolls over or an admin raises the cap. Enforcement lives at the two choke points every path funnels through, so replay, dev/bot triggers and Slack are all covered. Checking is eventually consistent — a small overshoot is accepted rather than paying for hard reservation on the hot path. - Spend rollup APIs: a workspace admin sees their own month (by job type and day); an instance admin sees a metadata-only rollup across all workspaces and sets the cap. - Admin UI: a per-workspace usage page and an instance-wide table with an inline budget editor, plus paused/over-budget and unpriced-model warnings. Cost is resolved and sanity-checked in one place (the recorder): the runtime's reported cost wins when plausible, else the model pricing table, else the row is stored uncosted and surfaced (count + metric + warning) so the cap's blind spot is visible rather than silent. Includes a Liquibase migration that creates the ledger, adds the budget column, and backfills both histories. Two refactors fell out of the module boundaries: agent-job cancel/retry split into `AgentJobLifecycleService`, and the cross-tenant admin read split into a `@WorkspaceAgnostic` service. Part of #1354. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 811 files, which is 711 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (877)
You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a unified LLM usage ledger with monthly workspace caps, enforcement for agent and mentor flows, audited budget administration, reporting APIs, and admin/workspace usage pages with generated client bindings and tests. ChangesLLM budget governance
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: ✨ Finishing Touches🧪 Generate unit tests (beta)
|
github-actions
Bot
added
documentation
Contributor
📚 Documentation Preview
|
…tion-readiness # Conflicts: # server/openapi.yaml # server/src/main/resources/db/master.xml # server/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobServiceTest.java # webapp/src/api/@tanstack/react-query.gen.ts # webapp/src/api/index.ts # webapp/src/api/sdk.gen.ts # webapp/src/api/transformers.gen.ts # webapp/src/api/types.gen.ts # webapp/src/components/core/sidebar/NavAdmin.tsx # webapp/src/routeTree.gen.ts # webapp/src/routes/_authenticated/w/$workspaceSlug/admin/usage.tsx
FelixTJDietrich
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
server/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobLifecycleService.java (1)
190-201: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffExternal/blocking calls run inside the
@Transactionalcancel.Unlike
retryDelivery, which deliberately pushes delivery outside the transaction,cancelperforms the WSSdispatch(...)andsandboxManager.cancel(...)while the method-level transaction is still open, holding a DB connection during a potentially slow container stop / network call. Cancel is admin-initiated and infrequent, so impact is limited, but consider moving the best-effort sandbox/worker signalling after the transaction commits for consistency with the delivery path.🤖 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 `@server/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobLifecycleService.java` around lines 190 - 201, Move the best-effort cancellation calls from the transactional cancel flow into an after-commit path, matching the existing retryDelivery pattern. Ensure workerJobCancelDispatcher.dispatch and sandboxManager.cancel run only after the database cancellation transition commits, while preserving their current no-op and exception-handling behavior.server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageEventRepository.java (1)
66-80: 🚀 Performance & Scalability | 🔵 TrivialConsider pagination for the cross-tenant admin rollup.
aggregateByWorkspace(and the/admin/llm-usageendpoint behind it) returns one row per workspace with no limit. Fine at small instance scale, but worth paging (similar to the existing/admin/config-auditpage/size pattern) before instance workspace counts grow, to avoid loading the full workspace table into memory on every admin usage view.🤖 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 `@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageEventRepository.java` around lines 66 - 80, Update LlmUsageEventRepository.aggregateByWorkspace and the underlying /admin/llm-usage flow to support page/size pagination, following the existing /admin/config-audit pattern. Apply pagination to the grouped workspace aggregate query while preserving the current date filtering, zero-usage LEFT JOIN behavior, and cost-descending order.server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java (1)
71-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew test methods across the ledger/budget test files don't follow the
should[ExpectedBehavior]When[Condition]naming convention. All three new test files name their test methods descriptively (e.g.recordAppendsOneLedgerRow,spendAtBudgetIsExhausted,overBudgetFlagFlipsWhenSpendReachesTheCap) rather than using the repository's mandated pattern.
server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java#L71-L214: rename methods such asrecordAppendsOneLedgerRow→shouldAppendLedgerRowWhenUsageIsRecorded,crossingTheBudgetFiresTheExhaustedCounterOnce→shouldIncrementExhaustedCounterOnceWhenBudgetIsCrossed, etc.server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmBudgetServiceTest.java#L51-L103: rename methods such asspendAtBudgetIsExhausted→shouldBeExhaustedWhenSpendEqualsBudget,windowIsHalfOpenUtcCalendarMonth→shouldReturnHalfOpenUtcWindowWhenGivenAMonth.server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageControllerIntegrationTest.java#L66-L165: rename methods such asreportRollsUpTheRequestedMonthByJobTypeAndDay→shouldRollUpUsageByJobTypeAndDayWhenMonthIsRequested,plainMemberIsForbidden→shouldReturnForbiddenWhenPlainMemberRequestsReport.As per path instructions, "Name tests using
should[ExpectedBehavior]When[Condition]."🤖 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 `@server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java` around lines 71 - 214, Rename every new test method to follow the should[ExpectedBehavior]When[Condition] convention. Update methods in server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java (lines 71-214), LlmBudgetServiceTest.java (lines 51-103), and LlmUsageControllerIntegrationTest.java (lines 66-165), preserving each test’s behavior and using names such as shouldAppendLedgerRowWhenUsageIsRecorded, shouldBeExhaustedWhenSpendEqualsBudget, and shouldReturnForbiddenWhenPlainMemberRequestsReport.Source: Path instructions
server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java (1)
46-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared month
@Pattern/parse logic. BothLlmUsageAdminController.listandLlmUsageController.getReportrepeat the identical"\\d{4}-(0[1-9]|1[0-2])"regex andYearMonth.parse(month) : YearMonth.now(ZoneOffset.UTC)fallback — a single shared constant/helper would prevent the two validators from drifting apart.
server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java#L46-L59: replace the inline@Patternregex andYearMonth.parse/nowfallback inlist(...)with a call to a shared helper (e.g. a smallMonthParamutility or a custom@ValidMonthannotation).server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageController.java#L36-L48: apply the same shared helper ingetReport(...).🤖 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 `@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java` around lines 46 - 59, Extract the duplicated month validation and parsing into one shared helper or value type, then update LlmUsageAdminController.list in server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java lines 46-59 and LlmUsageController.getReport in server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageController.java lines 36-48 to use it. The shared logic must retain the ISO yyyy-MM validation and UTC current-month fallback.server/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobExecutorTest.java (1)
61-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test verifies
usageRecorderis actually invoked.The mock is wired into every constructor call, but none of the tests (e.g.
shouldCompleteJobSuccessfully) assert thatusageRecorder.record(...)is called with the expected job/workspace/cost data after a completed run. This is exactly the new ledger-writing behavior this cohort adds toAgentJobExecutor.Want me to draft a
verify(usageRecorder).record(...)assertion for the success/failure paths onceAgentJobExecutor's exact call site is confirmed?Also applies to: 348-376
🤖 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 `@server/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobExecutorTest.java` around lines 61 - 63, Add assertions to AgentJobExecutorTest, especially shouldCompleteJobSuccessfully and the relevant failure-path test, verifying usageRecorder.record(...) is invoked with the expected job, workspace, and cost data after execution. Use Mockito verify against the existing usageRecorder mock and cover both successful and failed runs as applicable.server/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatMetrics.java (1)
110-114: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a dedicated
Outcome.BUDGET_BLOCKEDinstead of falling through toERROR.
recordBudgetBlocked()is a good specific signal, but the turn's terminal outcome (recorded inMentorChatService.dispatchTurn's genericcatch (RuntimeException e)) still lands inOutcome.ERROR, diluting that bucket with expected budget-cap rejections instead of real failures.🤖 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 `@server/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatMetrics.java` around lines 110 - 114, Add a dedicated Outcome.BUDGET_BLOCKED and update MentorChatService.dispatchTurn so budget-cap rejections are classified with that outcome rather than Outcome.ERROR. Ensure the generic RuntimeException path continues to record Outcome.ERROR for genuine failures, while the existing recordBudgetBlocked metric remains intact.
🤖 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 @.changeset/per-workspace-llm-budget.md:
- Around line 5-17: Add an “Operators:” section to the changeset explicitly
stating whether deployment, migration, or configuration action is required; if
none is needed, state that no operator action is required.
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmBudgetExhaustedException.java`:
- Around line 10-17: Update the LlmBudgetExhaustedException constructor message
to remove the raw workspaceId from the user-facing text, while preserving the
existing budget guidance; keep workspace identification available only through
server-side logging if needed.
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageService.java`:
- Around line 38-53: Update the aggregation mapping in LlmUsageService to
tolerate unknown or renamed job_type values instead of calling
LlmUsageJobType.valueOf(row.getJobType()) directly. Add the required database
validation or use an established fallback/nullable conversion so one
unrecognized value cannot fail the entire workspace usage report, while
preserving normal enum mapping for known values.
In
`@server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminControllerIntegrationTest.java`:
- Around line 58-148: Rename the four test methods in the admin usage
integration test to follow the should[ExpectedBehavior]When[Condition]
convention: adminSeesPerWorkspaceSpendIncludingZeroSpendWorkspaces,
adminSetsAndClearsTheBudgetCap, negativeBudgetIsRejectedWith400, and
nonAdminIsForbidden. Preserve each test’s behavior and assertions.
---
Nitpick comments:
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobLifecycleService.java`:
- Around line 190-201: Move the best-effort cancellation calls from the
transactional cancel flow into an after-commit path, matching the existing
retryDelivery pattern. Ensure workerJobCancelDispatcher.dispatch and
sandboxManager.cancel run only after the database cancellation transition
commits, while preserving their current no-op and exception-handling behavior.
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatMetrics.java`:
- Around line 110-114: Add a dedicated Outcome.BUDGET_BLOCKED and update
MentorChatService.dispatchTurn so budget-cap rejections are classified with that
outcome rather than Outcome.ERROR. Ensure the generic RuntimeException path
continues to record Outcome.ERROR for genuine failures, while the existing
recordBudgetBlocked metric remains intact.
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java`:
- Around line 46-59: Extract the duplicated month validation and parsing into
one shared helper or value type, then update LlmUsageAdminController.list in
server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.java
lines 46-59 and LlmUsageController.getReport in
server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageController.java
lines 36-48 to use it. The shared logic must retain the ISO yyyy-MM validation
and UTC current-month fallback.
In
`@server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageEventRepository.java`:
- Around line 66-80: Update LlmUsageEventRepository.aggregateByWorkspace and the
underlying /admin/llm-usage flow to support page/size pagination, following the
existing /admin/config-audit pattern. Apply pagination to the grouped workspace
aggregate query while preserving the current date filtering, zero-usage LEFT
JOIN behavior, and cost-descending order.
In
`@server/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobExecutorTest.java`:
- Around line 61-63: Add assertions to AgentJobExecutorTest, especially
shouldCompleteJobSuccessfully and the relevant failure-path test, verifying
usageRecorder.record(...) is invoked with the expected job, workspace, and cost
data after execution. Use Mockito verify against the existing usageRecorder mock
and cover both successful and failed runs as applicable.
In
`@server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java`:
- Around line 71-214: Rename every new test method to follow the
should[ExpectedBehavior]When[Condition] convention. Update methods in
server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java
(lines 71-214), LlmBudgetServiceTest.java (lines 51-103), and
LlmUsageControllerIntegrationTest.java (lines 66-165), preserving each test’s
behavior and using names such as shouldAppendLedgerRowWhenUsageIsRecorded,
shouldBeExhaustedWhenSpendEqualsBudget, and
shouldReturnForbiddenWhenPlainMemberRequestsReport.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a451215b-27e4-454f-8aeb-3cd1562bdb72
📒 Files selected for processing (54)
.changeset/per-workspace-llm-budget.mddocs/contributor/erd/schema.mmdserver/openapi.yamlserver/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobController.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobExecutor.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobLifecycleService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatMetrics.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorTurnPersistence.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmBudgetExhaustedException.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmBudgetService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminController.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageController.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageDTOs.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageEvent.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageEventRepository.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageJobType.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageRecorder.javaserver/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageService.javaserver/src/main/java/de/tum/cit/aet/hephaestus/core/audit/spi/ConfigAuditEntityType.javaserver/src/main/java/de/tum/cit/aet/hephaestus/workspace/Workspace.javaserver/src/main/resources/db/changelog/1784534949268_changelog.xmlserver/src/main/resources/db/master.xmlserver/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobExecutorTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobLifecycleServiceTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/job/AgentJobServiceTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorChatServiceTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/mentor/chat/MentorTurnPersistenceCostTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmBudgetServiceTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageAdminControllerIntegrationTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageControllerIntegrationTest.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.javawebapp/src/api/@tanstack/react-query.gen.tswebapp/src/api/index.tswebapp/src/api/sdk.gen.tswebapp/src/api/transformers.gen.tswebapp/src/api/types.gen.tswebapp/src/components/admin/config-audit/configAuditFormat.tswebapp/src/components/admin/usage/AdminInstanceLlmUsageTable.stories.tsxwebapp/src/components/admin/usage/AdminInstanceLlmUsageTable.tsxwebapp/src/components/admin/usage/AdminLlmUsagePage.stories.tsxwebapp/src/components/admin/usage/AdminLlmUsagePage.tsxwebapp/src/components/admin/usage/MonthNavigator.stories.tsxwebapp/src/components/admin/usage/MonthNavigator.tsxwebapp/src/components/admin/usage/SetBudgetDialog.stories.tsxwebapp/src/components/admin/usage/SetBudgetDialog.tsxwebapp/src/components/admin/usage/usageUtils.tswebapp/src/components/core/sidebar/NavAdmin.tsxwebapp/src/components/core/sidebar/NavSuperAdmin.tsxwebapp/src/routeTree.gen.tswebapp/src/routes/_authenticated/admin.usage.tsxwebapp/src/routes/_authenticated/w/$workspaceSlug/admin/usage.tsx
Comment on lines
+5
to
+17
| Instance administrators can now see what each workspace spent on AI in a given month, and set a | ||
| monthly spending cap per workspace. Once a workspace reaches its cap, practice detection and mentor | ||
| replies pause for the rest of the month — so one runaway workspace can no longer quietly consume the | ||
| whole instance's AI budget — and they resume on their own when the next month begins or when an | ||
| administrator raises the cap. Changes to a cap are recorded in the audit log alongside other | ||
| administrative changes. | ||
|
|
||
| Workspace administrators get a matching view for their own workspace under Administration → | ||
| "Usage": total spend for the month, a breakdown by day and by kind of work (pull-request reviews, | ||
| issue reviews, conversation reviews, and mentor conversations), and their current cap, which they can | ||
| see but not raise. Mentor conversations are included in these totals for the first time. Where a | ||
| model has no price on record, the affected calls are counted separately and flagged, so it is clear | ||
| when a total is understated rather than silently wrong. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required **Operators:** line.
The changeset must state any deployment, migration, or configuration action required from operators. If none is required, explicitly say so.
Proposed addition
Where a
model has no price on record, the affected calls are counted separately and flagged, so it is clear
when a total is understated rather than silently wrong.
+
+**Operators:** Run the standard database migrations; no additional configuration is required.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Instance administrators can now see what each workspace spent on AI in a given month, and set a | |
| monthly spending cap per workspace. Once a workspace reaches its cap, practice detection and mentor | |
| replies pause for the rest of the month — so one runaway workspace can no longer quietly consume the | |
| whole instance's AI budget — and they resume on their own when the next month begins or when an | |
| administrator raises the cap. Changes to a cap are recorded in the audit log alongside other | |
| administrative changes. | |
| Workspace administrators get a matching view for their own workspace under Administration → | |
| "Usage": total spend for the month, a breakdown by day and by kind of work (pull-request reviews, | |
| issue reviews, conversation reviews, and mentor conversations), and their current cap, which they can | |
| see but not raise. Mentor conversations are included in these totals for the first time. Where a | |
| model has no price on record, the affected calls are counted separately and flagged, so it is clear | |
| when a total is understated rather than silently wrong. | |
| Instance administrators can now see what each workspace spent on AI in a given month, and set a | |
| monthly spending cap per workspace. Once a workspace reaches its cap, practice detection and mentor | |
| replies pause for the rest of the month — so one runaway workspace can no longer quietly consume the | |
| whole instance's AI budget — and they resume on their own when the next month begins or when an | |
| administrator raises the cap. Changes to a cap are recorded in the audit log alongside other | |
| administrative changes. | |
| Workspace administrators get a matching view for their own workspace under Administration → | |
| "Usage": total spend for the month, a breakdown by day and by kind of work (pull-request reviews, | |
| issue reviews, conversation reviews, and mentor conversations), and their current cap, which they can | |
| see but not raise. Mentor conversations are included in these totals for the first time. Where a | |
| model has no price on record, the affected calls are counted separately and flagged, so it is clear | |
| when a total is understated rather than silently wrong. | |
| **Operators:** Run the standard database migrations; no additional configuration is required. |
🤖 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 @.changeset/per-workspace-llm-budget.md around lines 5 - 17, Add an
“Operators:” section to the changeset explicitly stating whether deployment,
migration, or configuration action is required; if none is needed, state that no
operator action is required.
Source: Coding guidelines
Native `min`/`step` constraint validation silently blocked submit before the form's own handler ran, so the field's explanation of *why* a value was rejected never rendered — the browser showed a bubble instead. The form now carries `noValidate` and owns every rule (empty, negative, sub-cent), so all rejections surface through the same `FieldError`. Caught by the Storybook test tier, which the earlier UI pass hadn't run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Found by deploying to the test instance: the runtime reported `costUsd: 0` for a job that burned 4.3M input tokens, and `PiResultParser` also defaults an *absent* cost to 0.0. The recorder trusted that zero, so real spend landed in the ledger as $0 and the monthly cap could never fire — the exact blind spot the ledger exists to remove. A zero alongside non-zero tokens now falls through to the model pricing table, and to the uncosted path when the model has no price on record. A genuine zero with no tokens is still recorded as zero. Verified live: the same review now resolves $0.513384 from the pricing table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
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
`@server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java`:
- Around line 199-232: Rename the affected test methods in
LlmUsageLedgerIntegrationTest to follow the
should[ExpectedBehavior]When[Condition] convention, including
reportedZeroCostWithRealTokensFallsBackToPricingInsteadOfBillingZero and
reportedZeroCostWithNoTokensStaysZero. Preserve each test’s existing behavior
and assertions.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 719c8ce5-12e3-4f48-80fa-e00fe8b26e65
📒 Files selected for processing (4)
server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageRecorder.javaserver/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.javawebapp/src/components/admin/usage/SetBudgetDialog.stories.tsxwebapp/src/components/admin/usage/SetBudgetDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- webapp/src/components/admin/usage/SetBudgetDialog.stories.tsx
- webapp/src/components/admin/usage/SetBudgetDialog.tsx
- server/src/main/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageRecorder.java
Comment on lines
+199
to
+232
| @Test | ||
| void reportedZeroCostWithRealTokensFallsBackToPricingInsteadOfBillingZero() { | ||
| Workspace workspace = setupWorkspace("ledger-zero-cost"); | ||
| ModelPricing pricing = new ModelPricing(); | ||
| pricing.setModelId("zero-reporting-model"); | ||
| pricing.setPer1kInputUsd(new BigDecimal("0.001000")); | ||
| pricing.setPer1kOutputUsd(new BigDecimal("0.002000")); | ||
| pricing.setPer1kCacheReadUsd(BigDecimal.ZERO); | ||
| pricing.setPer1kCacheWriteUsd(BigDecimal.ZERO); | ||
| pricing.setValidFrom(Instant.now().minusSeconds(60)); | ||
| pricingRepository.save(pricing); | ||
|
|
||
| recorder.record( | ||
| workspace.getId(), | ||
| new LlmUsageRecorder.LlmUsageSample( | ||
| LlmUsageJobType.PULL_REQUEST_REVIEW, | ||
| UUID.randomUUID(), | ||
| "zero-reporting-model", | ||
| 1000, | ||
| 1000, | ||
| 0, | ||
| 0, | ||
| 77, | ||
| BigDecimal.ZERO, // runtime reported "free" for a job that clearly was not | ||
| Instant.now() | ||
| ) | ||
| ); | ||
|
|
||
| assertThat(budgetService.monthToDateCost(workspace.getId())).isEqualByComparingTo("0.003000"); | ||
| } | ||
|
|
||
| /** A genuine zero — no tokens burned — is still recorded as zero, not re-derived. */ | ||
| @Test | ||
| void reportedZeroCostWithNoTokensStaysZero() { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename test methods to follow the should[ExpectedBehavior]When[Condition] convention.
The current test names do not comply with the repository's test naming convention. As per coding guidelines, tests should be named using the should[ExpectedBehavior]When[Condition] format.
♻️ Proposed rename
- void reportedZeroCostWithRealTokensFallsBackToPricingInsteadOfBillingZero() {
+ void shouldFallbackToPricingWhenReportedCostIsZeroAndTokensAreReal() {
Workspace workspace = setupWorkspace("ledger-zero-cost");
ModelPricing pricing = new ModelPricing();
pricing.setModelId("zero-reporting-model");
pricing.setPer1kInputUsd(new BigDecimal("0.001000"));
pricing.setPer1kOutputUsd(new BigDecimal("0.002000"));
pricing.setPer1kCacheReadUsd(BigDecimal.ZERO);
pricing.setPer1kCacheWriteUsd(BigDecimal.ZERO);
pricing.setValidFrom(Instant.now().minusSeconds(60));
pricingRepository.save(pricing);
recorder.record(
workspace.getId(),
new LlmUsageRecorder.LlmUsageSample(
LlmUsageJobType.PULL_REQUEST_REVIEW,
UUID.randomUUID(),
"zero-reporting-model",
1000,
1000,
0,
0,
77,
BigDecimal.ZERO, // runtime reported "free" for a job that clearly was not
Instant.now()
)
);
assertThat(budgetService.monthToDateCost(workspace.getId())).isEqualByComparingTo("0.003000");
}
/** A genuine zero — no tokens burned — is still recorded as zero, not re-derived. */
`@Test`
- void reportedZeroCostWithNoTokensStaysZero() {
+ void shouldStayZeroWhenReportedCostIsZeroAndNoTokensBurned() {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Test | |
| void reportedZeroCostWithRealTokensFallsBackToPricingInsteadOfBillingZero() { | |
| Workspace workspace = setupWorkspace("ledger-zero-cost"); | |
| ModelPricing pricing = new ModelPricing(); | |
| pricing.setModelId("zero-reporting-model"); | |
| pricing.setPer1kInputUsd(new BigDecimal("0.001000")); | |
| pricing.setPer1kOutputUsd(new BigDecimal("0.002000")); | |
| pricing.setPer1kCacheReadUsd(BigDecimal.ZERO); | |
| pricing.setPer1kCacheWriteUsd(BigDecimal.ZERO); | |
| pricing.setValidFrom(Instant.now().minusSeconds(60)); | |
| pricingRepository.save(pricing); | |
| recorder.record( | |
| workspace.getId(), | |
| new LlmUsageRecorder.LlmUsageSample( | |
| LlmUsageJobType.PULL_REQUEST_REVIEW, | |
| UUID.randomUUID(), | |
| "zero-reporting-model", | |
| 1000, | |
| 1000, | |
| 0, | |
| 0, | |
| 77, | |
| BigDecimal.ZERO, // runtime reported "free" for a job that clearly was not | |
| Instant.now() | |
| ) | |
| ); | |
| assertThat(budgetService.monthToDateCost(workspace.getId())).isEqualByComparingTo("0.003000"); | |
| } | |
| /** A genuine zero — no tokens burned — is still recorded as zero, not re-derived. */ | |
| @Test | |
| void reportedZeroCostWithNoTokensStaysZero() { | |
| `@Test` | |
| void shouldFallbackToPricingWhenReportedCostIsZeroAndTokensAreReal() { | |
| Workspace workspace = setupWorkspace("ledger-zero-cost"); | |
| ModelPricing pricing = new ModelPricing(); | |
| pricing.setModelId("zero-reporting-model"); | |
| pricing.setPer1kInputUsd(new BigDecimal("0.001000")); | |
| pricing.setPer1kOutputUsd(new BigDecimal("0.002000")); | |
| pricing.setPer1kCacheReadUsd(BigDecimal.ZERO); | |
| pricing.setPer1kCacheWriteUsd(BigDecimal.ZERO); | |
| pricing.setValidFrom(Instant.now().minusSeconds(60)); | |
| pricingRepository.save(pricing); | |
| recorder.record( | |
| workspace.getId(), | |
| new LlmUsageRecorder.LlmUsageSample( | |
| LlmUsageJobType.PULL_REQUEST_REVIEW, | |
| UUID.randomUUID(), | |
| "zero-reporting-model", | |
| 1000, | |
| 1000, | |
| 0, | |
| 0, | |
| 77, | |
| BigDecimal.ZERO, // runtime reported "free" for a job that clearly was not | |
| Instant.now() | |
| ) | |
| ); | |
| assertThat(budgetService.monthToDateCost(workspace.getId())).isEqualByComparingTo("0.003000"); | |
| } | |
| /** A genuine zero — no tokens burned — is still recorded as zero, not re-derived. */ | |
| `@Test` | |
| void shouldStayZeroWhenReportedCostIsZeroAndNoTokensBurned() { |
🤖 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
`@server/src/test/java/de/tum/cit/aet/hephaestus/agent/usage/LlmUsageLedgerIntegrationTest.java`
around lines 199 - 232, Rename the affected test methods in
LlmUsageLedgerIntegrationTest to follow the
should[ExpectedBehavior]When[Condition] convention, including
reportedZeroCostWithRealTokensFallsBackToPricingInsteadOfBillingZero and
reportedZeroCostWithNoTokensStaysZero. Preserve each test’s existing behavior
and assertions.
Source: Coding guidelines
Foundation for the LLM configuration redesign (#1368, scope-expanded). Introduces two credential-owning scopes for talking to a model, both first-class: - INSTANCE catalog (app_admin-owned, GLOBAL): llm_connection -> llm_model -> temporal llm_model_price, with llm_model_workspace_grant for per-workspace sharing. - WORKSPACE catalog (tenant-scoped, workspace-admin-owned): workspace_llm_connection -> workspace_llm_model (inline price). Isolation enforced by same-workspace composite FKs. agent_config is demoted to a binding (instance_model_id XOR workspace_model_id). The usage ledger gains pricing_state (PRICED/FREE/UNPRICED — "unknown" stops silently counting as $0), reasoning_tokens, funding_source, and applied-price provenance. LlmModelResolver collapses instance / workspace / legacy configs into one non-secret ResolvedLlmModel; the credential is resolved live and never frozen or logged. There is one credential path only — no egress toggle; the key never enters the sandbox. Migration validated on a fresh Postgres (836 changesets); tenancy + data-isolation arch tests green. Additive and auto-applied; no operator action. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instance-admin (app_admin) CRUD for provider connections under /admin/llm/connections: create/list/get/update/delete + a "test & fetch models" probe. The write-only API key is never serialized (hasApiKey + last4 only); delete is refused with 409 while a model still references the connection. EgressPolicy is the shared SSRF guard reused by the probe (and later the proxy): every base URL must be a public HTTPS host, private/link-local/metadata addresses are blocked, and when the instance egress allowlist is set the host must be on it. The probe validates egress first, uses a dedicated 5s client, and is advisory — a 4xx/timeout returns reachable=false, never an error, and never the raw upstream body. Also adds GET/PUT /admin/llm/settings (egress allowlist, workspace-BYO toggle, default unpriced policy). Corrects the foundation changelog: the catalog subjects were wrongly added to the workspace-scoped config_audit CHECK — config-audit needs a workspace id, so instance-global catalog changes audit to auth_event instead (wiring in a later slice). Compile + audit-by-default arch test green; migration re-validated (835 changesets). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instance-admin (app_admin) CRUD for catalog models under /admin/llm: create on a connection (per-connection slug, 409 on conflict incl. the concurrent-create race), list/get with batched current-price + grant projections (no N+1), metadata update, and delete guarded by a 409 while any agent_config still binds the model. Pricing is temporal supersede-on-insert: repricing closes the open llm_model_price row and inserts the new open row in one transaction (ux_llm_model_price_open holds the invariant). PRICED requires input+output per-1M rates; FREE/UNPRICED reject rates, and FREE requires a funding note. Sharing is one "Share with" operation: all workspaces (clears grants) or a selected set (grant diff, unknown ids 400). 14 new unit tests (supersede ordering, pricing validation, sharing diff, in-use guard); architecture tier green (216/216). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dels Workspace-admin surface for the two-scope LLM catalog (#1368): - BYO connection CRUD + probe under the workspace scope, gated by the instance allow_workspace_connections setting (403 when off). Key is write-only (hasApiKey + last4), base_url passes the shared EgressPolicy SSRF guard, and the probe answers workspace-framed — reachable + model count, never a raw model dump. Delete refuses with 409 while a model references the connection. - BYO model CRUD with inline pricing; PRICED/FREE/UNPRICED validation extracted into LlmPriceValidation and shared with the instance catalog service. - agent_config binding: set exactly one of instanceModelId / workspaceModelId (both = 400); bind-time validation requires the instance model to be visible (public or granted), enabled, on an enabled connection, and the workspace model to belong to the calling workspace. Legacy (null,null) rows tolerated. - Available-models projection: union of visible shared models and own BYO models behind one deliberately narrow DTO — display names + price framing only, never upstream ids, base URLs, or auth material. 106 unit tests green; architecture tier green (tenancy, audit-by-default, snapshot secret-detector, dependency ceilings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The runtime now speaks to models exclusively through the two-scope catalog (#1368): one credential path, key never in the sandbox. - ConfigSnapshot v4 freezes resolved behaviour at dispatch (api protocol, base URL, upstream model id, capability, connection ref); the credential is resolved live per proxy request, so rotation and revocation reach in-flight jobs. Pre-v4 payloads are translated explicitly, not Jackson-defaulted. - LlmProxyController drops the /internal/llm/{provider} path segment — the connection comes from the authenticated job token (mentor turns use a process-local credential registry), so a sandbox can no longer pick which instance credential the proxy injects. EgressPolicy re-checked per request; Azure param quirks and stream_options.include_usage keyed on api protocol. - Deleted the last "key in sandbox" paths: CredentialMode, ProviderProxyConfig, LlmProxyAuthShell, LlmProxyProperties, WorkerProperties.Llm and the AgentJobExecutor override branch. Proxy beans gate on job-execution capability — "jobs on, proxy off" is unexpressible. - One shared pi-provider.mjs registration for both runners, driven by a per-job pi-provider.json (kills the cacheControlFormat / context-window / cost:{0,0,0,0} drift). PiResultParser now records reasoning tokens. Unit tier 5233/5233, architecture tier 216/216 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The usage ledger is now the single cost authority (#1368): - LlmUsageRecorder derives every cost server-side from the bound catalog price (instance open price row, BYO inline rates, or legacy model_pricing fallback); the runner-reported-cost branch and COST_USD_SANITY_CAP are gone. Every priced event freezes the applied per-1M rates (and price row / BYO model refs) onto the event, so a dollar stays falsifiable after repricing. cost_usd is guarded for NUMERIC(12,6): HALF_EVEN to 6dp, non-zero never truncates to zero, >=1e6 clamps with a WARN. - pricing_state per event: PRICED, FREE (declared, costs 0), or UNPRICED (cost NULL — unknown no longer masquerades as $0). - Budget verdict {WITHIN, EXHAUSTED, UNVERIFIABLE}: the cap compares only priced, instance-funded spend; BYO spend is reported separately and never capped. Unpriced instance-funded usage in the window makes the month UNVERIFIABLE — surfaced and logged, never blocking (v1 warns, not fails). Job-type and daily breakdowns carry the same split. - Dead Pi-reported cost path deleted (the runner structurally cannot price anymore); the mentor turn estimate keeps its ModelPricingService fallback, and agent_job.llm_cost_usd is no longer written with a fake $0. Migration changeset validated on a fresh Postgres (836 changesets clean). Unit tier 5249/5249, architecture tier 216/216 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every LLM catalog mutation is now recorded in the ledger that matches its scope (#1368): - Instance-global actions (connections, models, pricing, sharing, settings) write auth_event entries — nine new event types — through narrow SPI ports implemented by one server-role adapter, keeping the Modulith boundary to core.auth intact. Detail payloads carry ids and non-secret fields only. - Workspace BYO mutations record config_audit entries with two new entity types; connection snapshots redact the API key to a set/unset boolean and strip userinfo/query from base URLs, passing the snapshot secret detector. - Probe endpoints are @AuditExempt (they test a credential, store nothing); instance controllers now carry "auth_event LLM_*" markers, satisfying the audit-by-default rule with real wiring instead of placeholders. - Both CHECK constraints (auth_event type, config_audit entity type) widened in the branch changelog; validated on a fresh Postgres (838 changesets). Unit tier 5259/5259, architecture tier 216/216 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Regenerates server/openapi.yaml and the webapp TS client for the new catalog, BYO, available-models, and reshaped usage endpoints. Fourteen catalog request/result records were renamed to the mandatory *DTO suffix — the spec post-processor drops non-DTO schemas while paths still reference them, which broke client generation with unresolved refs. Webapp mechanically repaired to the new contract: usage pages read pricedTotalCostUsd / byoTotalCostUsd / unpricedEventCount / verdict, render the "at least $X — some usage has no price set" framing and an Unverified badge; audit-log label maps gained the nine new instance event types and the two workspace config-audit entity types. Typecheck, 418 webapp tests, build, and format all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…icker
Instance admins get an "AI models" console section (/admin/llm): provider
connections with a write-only key field and an advisory "Test & fetch models"
probe that never gates saving, per-connection models with capability fields,
a price editor (per-1M rates / Free with a required note / No price set), a
"Share with" all-or-selected control, and instance settings (allowed provider
hosts, workspace own-provider toggle).
Workspace admins pick models instead of typing provider credentials: the
agent-config form's hardcoded provider list is gone, replaced by a picker
grouped into "Shared models" and "Your provider" with price framing; legacy
unbound configs show a read-only hint. A new "Your AI provider" tab lets a
workspace connect its own provider ("Connected — N models available", never
a raw model dump) and respects the instance-level disable. Usage screens
render own-provider spend outside the budget bar and use the "Budget
reached" / "at least $X" wording.
Typecheck, 419 webapp tests, 913 Storybook interaction tests, build, and
format all green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
51 controller integration tests (instance catalog CRUD incl. key redaction and price supersede, workspace BYO tenancy and kill-switch, available-models leak assertions, binding validation) uncovered and fix two real defects: DTO conversion after the transaction closed threw LazyInitializationException (500) on model reads — repositories now JOIN FETCH the connection — and the four 409 conflict guards were shadowed to 500 by the global catch-all advice, fixed with a dedicated highest-precedence catalog advice. Creating an agent config with a model binding no longer demands a vestigial legacy provider value: either a binding or the legacy provider is required, and the webapp stops sending a placeholder. Interactive mentor sandboxes now revoke their proxy credential on every close path instead of letting it live out a 12h TTL. The e2e setup script drops the removed credentialMode field. Unit tier 5265/5265, architecture 216/216, targeted integration 51/51 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…talog Removes dead LLM env wiring (LLM_PROXY_*, HEPHAESTUS_WORKER_LLM_*) from the compose files and env examples, points runtime-role and setup docs at the admin console's AI models section, amends ADRs 0005/0006 per their own amendment precedents, adds the MIGRATION.md entry for the removed variables, regenerates the ERD (seven new llm tables + the agent_config binding), and ships the release changeset for the catalog + honest-budget feature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial review of the catalog branch found five exploitable gaps, all closed here: - EgressPolicy rejects provider URLs carrying userinfo, query strings, or fragments (a ?api-key=... gateway URL would otherwise ride into snapshots and logs), and the loopback exception is now opt-in via hephaestus.llm.egress.allow-loopback (default false) instead of an unconditional production SSRF hole. The guard finally has behavioral tests — 34 table-driven cases. - The connection probe no longer follows redirects, so a 302 from a probed host can't walk x-api-key/api-key headers to another origin. - The proxy resolves base URL and credential together from the live connection row (the frozen snapshot pins only the connection identity), so repointing a connection can no longer send the new key to the old host. - A keyless connection on an enabled, egress-valid host now forwards without auth injection instead of failing with 502 — self-hosted vLLM/Ollama gateways work as advertised. - Workspace-visible job DTOs redact an instance connection's base URL to scheme://host, AgentJob.toString() excludes the snapshot, and proxy logs carry hosts, not full URLs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review found the legacy backfill actively harmful: it defaulted every blank base URL to api.openai.com (sending legacy Anthropic keys to OpenAI), and rebinding legacy configs as workspace-funded exempted every pre-existing workspace from the budget cap this release introduces. The backfill binding is removed entirely — legacy configs stay unbound and the runtime fallback carries them with per-provider defaults and instance funding, which is what MIGRATION.md now accurately promises. Also: the mentor-history cost/token JSON casts are guarded so one malformed metadata row can no longer abort the whole migration; CHECK-widening changesets get empty rollbacks (re-narrowing fails once new-type rows exist); (connection_id, upstream_model_id) is now unique per scope with a friendly 409, closing the duplicate-upstream-id billing ambiguity; and cost_usd widens to NUMERIC(18,6) so the recorder no longer silently clamps a huge event downward. Validated on a fresh Postgres: 840 changesets clean, rollback/reapply of the tail changesets clean, malformed-JSON battery produces NULLs, duplicate insert rejected by the new unique index. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The unverifiable month renders as a warning line ("Some usage has no price
set") instead of a badge, agent-config cards use the locked "Model for
Practice detection" / "Model for Mentor" task framing, and a workspace whose
budget is exhausted now shows a "Budget reached" alert on the AI models page
itself — detection no longer pauses invisibly for admins who never open the
usage screen. Operator docs gain a workspace-budget section (set/clear
semantics, what pauses at the cap, the "at least $X" framing, and the
llm.budget.* metrics as alerting hooks, incl. the mentor reasoning-token
limitation); a stale runner comment naming a nonexistent test now describes
the real by-construction guarantee.
419 webapp tests and 914 Storybook interaction tests green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This branch had accumulated ~12,000 lines of added comment. Much of it restated the code, counted sibling files, or narrated a workaround instead of removing it. Server main sources lose 41% of their added commentary, the webapp 28%. Where a comment was carrying something real, the fix was usually to put it in the code instead: a named predicate, an extracted method, a type that makes the state it described unrepresentable. Where a comment explained a workaround, the workaround went with it. What survives is the part a reader cannot derive: an ordering requirement, a transaction boundary, a money-exactness rule, a security reason, or a real external constraint that would otherwise be "simplified" into a bug. No behaviour change. 5683 unit, 223 architecture, 682 webapp and 1043 Storybook tests pass unchanged.
Cutting the commentary in half meant reading every claim it made. Several were false, and three of those were hiding real defects. - **A workspace could lose control of providers it owns.** When an instance turned own-providers off, a workspace that had already connected one lost the whole section — no Edit, no Test, no Disconnect — while the banner beside it promised those keep working. Only the mutations were ever gated server-side, so the section now always mounts. - **A rate-limit gauge read zero forever.** No GraphQL document selects the field it was bound to, so it decoded to zero and overwrote the correct value seeded from REST. - **The retired-env-var warning could not fire.** It watched dotted property names for variables that never had the prefix Spring needs, so three of the four were unreachable — and its test passed because it set the dotted form directly. - The GDPR record of processing claimed no component writes an HTTP access log. nginx does, and one route carries a contributor's login in the URL. It also understated retention coverage and mis-stated when sandboxes run. Test commentary drops 49% and webapp 50%. Fifty-four tests were renamed to carry what their javadoc used to, and where a comment described a workaround, the workaround went with it. New: stubbed GraphQL responses are now validated against the checked-in schema, so a test can no longer assert against a reply the vendor could never send — the defect class behind the merge-request approval that had never once worked.
The documentation pressure test found ten places where a page existed to warn about something the code got wrong. Each is fixed at the source, and the paragraph it needed goes with it. - **No layer writes an HTTP access log any more.** nginx did — one line per request, and one route carries a contributor's login in the URL — while the record of processing said nothing did. Both nginx containers now disable it at the server level, so the strong claim is restored rather than qualified. - **A held run no longer looks queued.** The queue exposes why a job is parked and when it is next due, a new agent.queue.held metric separates "paused" from "idle", and the runs table and detail panel show both. The runbook step that told operators to go and check another page is gone. - **The retired-setting warning can actually fire.** It watched dotted property names for variables that never had the prefix Spring needs, and its test passed because it set those names by hand. It now watches the real variable names, compose forwards them for one release so a stale value is visible, and the test resolves them the way a container would. - **Feature flags work outside the shipped compose files**, the GitLab server follows your GitLab login instead of a pinned default, and the encryption-key error says how many bytes it got and from how many characters. Documentation: one page owns the install and one owns integrations; a new page documents connecting an AI provider from the controllers, with a worked pricing example. ADR 0025 loses the fix-log it had accumulated. Every orphaned page is in a sidebar, and the practice catalogue's counts now match the catalogue. The GraphQL commit-enrichment query moved into a checked-in document, closing the last place a query escaped both schema guards.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| AgentJobRepository agentJobRepository, | ||
| MentorProxyCredentialRegistry mentorRegistry, | ||
| ObjectMapper objectMapper | ||
| ) throws Exception { |
Static analysis flags the csrf.disable() on the proxy filter chain and cannot see what makes it safe: the chain is stateless and its filter reads a credential only from the Authorization header, so a forged cross-site request has nothing ambient to ride on. That was an argument in a comment; it is now three tests. The negative cases send a token the third case proves is good, so "authenticates nobody" cannot pass for the trivial reason that the token was bad, and they assert the repository is never consulted at all. The alert itself predates this branch and is open on main for this same file and three others.
The test deployment's nginx config disabled the access log only for /assets/, so every page request was logged — including /w/<slug>/user/<username>, which names a person, on an internet-exposed route. Same defect the shipped webapp config had; same fix, at server level where it covers everything.
This was referenced Jul 28, 2026
Closed
Closed
FelixTJDietrich
added a commit
that referenced
this pull request
Jul 28, 2026
…nd AI-config work main has since rewritten SECURITY.md and MIGRATION.md (#1394, #1392, #1400), so this branch's versions of both are dropped in favour of theirs — they say the same things, better. What remains is the public-surface repositioning. Also carries the changeset the release gate now requires, points the README's workspace bullet at the shipped AI-model configuration, and replaces the invented features in the FeatureCard stories with the shipped ones.
FelixTJDietrich
added a commit
that referenced
this pull request
Jul 28, 2026
Integrates the console shell with what landed on main since this branch opened. Silent mode now satisfies main's evaluation contract from #1402: a withheld unit must record WHY, or an evaluator cannot tell "model missed" from "policy withheld". Adds FeedbackSuppressionReason.INSTANCE_SILENCED and records it from both delivery gates, so suppressed feedback is auditable instead of invisible. The toggle is now audited. #1401's audit-by-default rule requires every admin mutation endpoint to declare its audit status; PUT /admin/settings/silent-mode records SILENT_MODE_CHANGED on the auth_event trail (instance-level, so not config_audit_event, whose workspace_id is NOT NULL). An incident review can now see who silenced the instance and why. Console nav absorbs #1400's new surfaces: AI models and AI usage join the grouped sidebar (Access / AI / Operations) instead of main's flat list, reusing the shared ADMIN_NAV_LABELS so both consoles name things the same way. The audit module moved on main (auditFormat.ts -> audit-format.ts plus audit-shared/), so the overview's activity card now uses refLabel and the shared self-ticking RelativeTime component rather than local helpers. The changelog is renamed to keep timestamps monotonic and its include appended last, per the changelog-immutability gate added in #1390. It also widens chk_feedback_suppression_reason for the new value. Re-verified on a fresh Postgres: 864 changesets apply, the seed row reads back, and there is no drift.
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Today a Hephaestus instance can only run AI against the providers we hard-coded, and nobody can see what that AI costs. A single busy workspace — or one runaway sync — can quietly spend the whole instance's budget, and the first sign of it is the provider bill.
This PR makes AI spend visible and bounded, and opens the model choice up.
Connect any OpenAI-compatible model. An instance admin registers a provider by base URL and API key, adds the models it serves, and shares them with the workspaces that should have them. A workspace that would rather pay its own provider can connect one itself. Self-hosted gateways and university GPU clusters work the same way as commercial APIs — there is no provider allowlist any more, just a URL that speaks the OpenAI wire format.
See what it costs. Every AI call now lands in one ledger, whether it came from practice detection or from a mentor conversation. Workspace admins get a monthly report broken down by day and by kind of work; instance admins get the same across every workspace. Amounts can be shown in a second currency alongside USD, converted at the European Central Bank's published reference rate.
Cap it. There are two budgets, and they never mix:
Keeping them apart is the point: a host that has run out of budget must not be able to stop work a workspace is paying for itself, and a workspace must never be able to spend the host's money by raising its own limit.
Spend counts while work is happening, not only when it finishes. That matters more than it sounds — a single agent run or mentor conversation makes many provider calls, so a cap that only sees completed work can be passed by a wide margin before it notices.
Fixes #1368
Reviewing this
It is a large diff. In rough order of how much it matters:
server/…/agent/usage/— the ledger, the two budgets, and the rule that decides whether a call may proceed.LlmBudgetServicestates the policy once, in prose, at the top.server/…/agent/proxy/— where a call is metered as it is served, and where the cap is re-checked before each forward.server/…/db/changelog/1785015307013_changelog.xml— one changelog. The interesting part is the migration that carries existing configurations into the new model catalogue before the old table is dropped.webapp/…/components/admin/usage/— the reports, the meters and the currency conversion.Three decisions are written down rather than left to be inferred from the code:
docs/contributor/llm-cost-vocabulary.md— price, cost, spend, cap and budget are five different numbers, and the UI is strict about which word appears where.How to test
Automated. CI covers the unit, architecture and integration tiers, plus the webapp and Storybook suites. Locally: 5,683 unit and 223 architecture tests, 682 webapp unit and 1,044 Storybook interaction tests, all passing.
The migration has its own integration test that builds the pre-upgrade schema, seeds representative legacy rows, runs the upgrade, and asserts that every endpoint, model name and encrypted key survived — then re-runs it and rolls it back. The changelog was additionally applied to a clean PostgreSQL 17 instance (861 change sets) to confirm it runs outside the test harness, since the test tiers use
ddl-auto: createand never execute Liquibase.By hand, against any OpenAI-compatible endpoint:
Also worth a look: set
HEPHAESTUS_LLM_DISPLAY_CURRENCY=EURand reload the usage page — every amount gains a second-currency estimate that states the rate and the date it was published. Open a past month and the estimate says it is frozen. Narrow the browser to 320 px and the dialogs still fit.What was verified live
Both paths were run end to end against a real self-hosted OpenAI-compatible model and a real GitLab instance, not mocks:
SANDBOX_MEMORY_BYTES.)The GitHub and GitLab comment-lookup documents added here were also executed against both live APIs, walking a real thread backwards and finding a real marker on the first request.
Fixes you may not expect to find in a cost-control PR
Working on the metering path surfaced defects around it. Each one either loses money, hides a change, or was outright broken:
AGENT_ENABLEDnow defaults tofalseon every runtime role. A worker started with the documented minimum environment used to claim jobs and spend money the operator believed was inert.Known limits
Stated plainly, because a cap described as tighter than it is would be worse than no cap.
The cap is eventually consistent, and the overshoot is bounded but not zero. The issue accepts this ("a small overshoot is acceptable"); ADR 0026 derives the bound and lists what it does not cover:
stream_options.include_usagecontributes nothing to the in-flight term; it is billed when the call completes.Other boundaries:
EURis the only display currency today. An unsupported value now fails startup naming what is accepted, rather than booting and silently showing USD only.RequestedReviewerunion conflicts with the spec's field-merging rule (User.name: StringvsTeam.name: String!). GitHub does not enforce it and the documents run; the waiver is scoped to that message on those paths, and any other conflict fails the build.agent-pionly — deliberately. It is the one image resolved at runtime rather than at deploy time, its digest is written by the release workflow, and a boot guard rejects anything that is not a digest. Nothing maintains a digest for the other images, and a pin nothing updates silently freezes a service on unpatched bytes. Self-hosters get an exact, CI-enforced release tag instead.One change reads like a regression and is not:
docs/runbooks/auth-cutover.mdnow says JWK rotation is not implemented. It never was — the previous text described arotate()method that does not exist. This replaces a false document with an accurate one.Upgrade notes
The database migration runs automatically. Existing AI configurations are carried into the new catalogue — endpoints, model names and encrypted keys included — and land disabled, because the endpoint some of them used lived in an environment variable the migration cannot read. Re-enable them once you have checked the endpoint is the one you expect. Any configuration that could not be carried over is named in the deploy log while the old table still exists, so nothing is lost silently.
MIGRATION.mdwalks through it step by step, and 13 of the 52 changesets carry an explicit Operators: line.Checklist
.changeset/README.md**Operators:** …) andMIGRATION.mdis updatedScreenshots
Not attached. The admin screens are covered by Storybook — see the Chromatic build linked in the checks for the usage report, the budget meters, the currency disclosure and the mobile layouts.