feat(server): inject contributor practice history into agent context - #912
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdded contributor practice history aggregation and injection into agent context. Introduced Changes
Sequence DiagramsequenceDiagram
participant Agent as Pull Request Agent
participant Handler as PullRequestReviewHandler
participant Repo as PracticeFindingRepository
participant Provider as ContributorHistoryProvider
participant Context as .context/ Files
Agent->>Handler: prepareInputFiles(jobRequest)
Handler->>Repo: Load PR entity (by ID)
Repo-->>Handler: PullRequest
Handler->>Handler: buildPullRequestMetadata(PR)
Handler->>Context: Write metadata.json
alt Has author && workspace
Handler->>Provider: buildHistoryJson(contributorId, workspaceId)
Provider->>Repo: findContributorPracticeSummary(...)
Repo-->>Provider: List<ContributorPracticeSummary>
alt Has findings (not all NOT_APPLICABLE)
Provider->>Provider: Aggregate, sort, cap (MAX_PRACTICES=20)
Provider->>Provider: Serialize to JSON
Provider-->>Handler: Optional<byte[]>
Handler->>Context: Write contributor_history.json
else No relevant findings
Provider-->>Handler: Optional.empty()
end
end
Handler->>Handler: verifyActivePractices(workspace)
Handler->>Handler: buildPrompt()
Handler->>Context: Write prompt.txt
Handler-->>Agent: Context prepared with history guidance
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
application-server
Contributor
There was a problem hiding this comment.
Pull request overview
Adds contributor practice verdict history as optional, aggregated context for the PR review agent, enabling better calibration of Cognitive Apprenticeship guidanceMethod based on prior outcomes.
Changes:
- Add a JPQL aggregate query + projection to summarize a contributor’s historical practice verdicts within a workspace.
- Introduce
ContributorHistoryProviderto transform the summary into a compact, capped JSON array and inject it into the agent sandbox as.context/contributor_history.json. - Wire the provider into
PullRequestReviewHandlerand extend unit/integration tests for repository aggregation and handler injection behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorPracticeSummary.java | New Spring Data projection for (practiceSlug, verdict, count, lastDetectedAt) aggregate rows. |
| server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProvider.java | New transformer to filter/aggregate/sort/cap history and serialize it to JSON bytes. |
| server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/PracticeFindingRepository.java | Add findContributorPracticeSummary() JPQL aggregation query. |
| server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandler.java | Load PR once, inject contributor history file, and update prompt instructions to reference it. |
| server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/JobTypeHandlerConfiguration.java | Register ContributorHistoryProvider bean and wire into PR review handler. |
| server/application-server/src/test/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProviderTest.java | New unit tests validating JSON shape, filtering, sorting/capping, and serialization failure behavior. |
| server/application-server/src/test/java/de/tum/in/www1/hephaestus/practices/finding/PracticeFindingRepositoryIntegrationTest.java | Add integration tests validating aggregation correctness and scoping (workspace/contributor). |
| server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandlerTest.java | Add tests for history file injection/omission and graceful handling of provider exceptions. |
| server/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/handler/JobTypeHandlerRegistryTest.java | Update handler construction to include the new dependency. |
| sb.append("Use this to calibrate your `guidanceMethod`: if a contributor has repeated NEGATIVE "); | ||
| sb.append("findings for a practice, escalate from MODELING to COACHING or SCAFFOLDING. "); | ||
| sb.append("If they show sustained POSITIVE verdicts, use REFLECTION or EXPLORATION. "); | ||
| sb.append("If the file is absent, this is a new contributor — default to COACHING.\n"); |
There was a problem hiding this comment.
The prompt implies that a missing /workspace/.context/contributor_history.json means “this is a new contributor”, but the file can also be absent due to DB/serialization failures or skipped enrichment. This can mislead guidanceMethod selection; consider wording it as “history unavailable” and defaulting to COACHING without asserting the contributor is new.
Suggested change
| sb.append("If the file is absent, this is a new contributor — default to COACHING.\n"); | |
| sb.append("If the file is absent, contributor history is unavailable — default to COACHING.\n"); |
Comment on lines
+606
to
+607
| * <p>Skips silently if the PR has no author or the contributor has no prior findings — | ||
| * the absence of the file signals a first-time contributor to the agent. |
There was a problem hiding this comment.
The Javadoc states that absence of contributor_history.json “signals a first-time contributor”, but in this implementation the file is also absent when the provider returns empty due to serialization errors or when the handler catches exceptions. Consider adjusting the comment so it doesn’t claim absence == new contributor.
Suggested change
| * <p>Skips silently if the PR has no author or the contributor has no prior findings — | |
| * the absence of the file signals a first-time contributor to the agent. | |
| * <p>Skips silently if the PR has no author or if no contributor history is available. | |
| * In that case, {@code .context/contributor_history.json} is simply omitted and the agent | |
| * should treat the absence as "no prior history is available" (which can include, but is | |
| * not limited to, first-time contributors or failures while building the history). |
| PullRequest pullRequest = pullRequestRepository.findByIdWithAllForGate(pullRequestId).orElse(null); | ||
| if (pullRequest == null) { | ||
| log.warn("Pull request not found in database during context preparation: pullRequestId={}", pullRequestId); | ||
| log.warn("Pull request not found in database during context preparation"); |
There was a problem hiding this comment.
This warn log lost the pullRequestId context (it’s no longer included in the message or MDC). Including the PR ID (and ideally workspace/repo) here would make diagnosing missing-PR enrichment much easier when scanning logs.
Suggested change
| log.warn("Pull request not found in database during context preparation"); | |
| log.warn( | |
| "Pull request not found in database during context preparation (pr_number={}, repository_full_name={})", | |
| requireInt(jobMetadata, "pr_number"), | |
| requireText(jobMetadata, "repository_full_name")); |
| return Optional.of(json); | ||
| } catch (JsonProcessingException e) { | ||
| // Should never happen for ObjectNode serialization | ||
| log.error("Failed to serialize contributor history JSON", e); |
There was a problem hiding this comment.
On serialization failure, this error log doesn’t include contributorId/workspaceId, and the caller will silently treat the history as absent. Add contextual identifiers to the log message (and optionally the practice count/byte size) so operators can correlate failures to specific jobs/users.
Suggested change
| log.error("Failed to serialize contributor history JSON", e); | |
| log.error( | |
| "Failed to serialize contributor history JSON for contributorId={}, workspaceId={}, practices={}", | |
| contributorId, | |
| workspaceId, | |
| sorted.size(), | |
| e | |
| ); |
Comment on lines
+545
to
+549
| when(pullRequestRepository.findByIdWithAllForGate(456L)).thenReturn(Optional.of(pullRequest)); | ||
| when(contributorHistoryProvider.buildHistoryJson(42L, WORKSPACE_ID)).thenThrow( | ||
| new RuntimeException("DB connection timeout") | ||
| ); | ||
|
|
There was a problem hiding this comment.
This test stubs contributorHistoryProvider.buildHistoryJson() to throw, but doesn’t verify that the method was actually invoked. As written, it could still pass if contributor history injection is accidentally skipped. Add a verify(contributorHistoryProvider).buildHistoryJson(42L, WORKSPACE_ID) (or similar) to assert the intended interaction.
Give the practice-aware review agent aggregated verdict history per contributor so it can calibrate Cognitive Apprenticeship guidance methods (MODELING → COACHING → SCAFFOLDING) instead of guessing in isolation each review. Closes #895 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FelixTJDietrich
force-pushed
the
feat/contributor-practice-history
branch
from
March 25, 2026 07:22
7e8f776 to
11167ac
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandler.java (1)
643-661: Consider caching or removing duplicate practice query.
verifyActivePractices()queriesfindByWorkspaceIdAndActiveTrue(workspaceId), butbuildPrompt()(lines 198-203) performs the same query independently. While the query is cheap and the methods may be called at different lifecycle stages, if they're always called together (as in the current flow), this results in a redundant database round-trip.If acceptable, this can be deferred. Otherwise, consider either:
- Caching the result in job preparation context
- Returning the practices from
verifyActivePractices()and passing them to a later method🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandler.java` around lines 643 - 661, The verifyActivePractices(AgentJob) method currently performs the same practiceRepository.findByWorkspaceIdAndActiveTrue(workspaceId) query that buildPrompt(...) performs later; avoid the duplicate DB call by having verifyActivePractices return the List<Practice> (or store it on the AgentJob/prep context) and pass that List into buildPrompt so buildPrompt uses the already-fetched practices instead of re-querying. Update verifyActivePractices signature to return List<Practice> (or set a field on AgentJob), adjust callers in PullRequestReviewHandler to accept the returned practices (or read from the job/context), and remove the redundant findByWorkspaceIdAndActiveTrue call from buildPrompt.server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProvider.java (1)
28-41: Align this provider with app-server conventions (@Service, Lombok constructor injection,@Slf4j).Implementation is correct, but this class currently bypasses repository conventions by using manual constructor wiring and
LoggerFactory. Consolidating on Lombok +@Servicekeeps consistency with the rest of the application server.Proposed refactor
+import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Service; -public class ContributorHistoryProvider { - - private static final Logger log = LoggerFactory.getLogger(ContributorHistoryProvider.class); +@Service +@Slf4j +@RequiredArgsConstructor +public class ContributorHistoryProvider { /** Maximum number of practices included in the history JSON. */ static final int MAX_PRACTICES = 20; private final PracticeFindingRepository practiceFindingRepository; private final ObjectMapper objectMapper; - - public ContributorHistoryProvider(PracticeFindingRepository practiceFindingRepository, ObjectMapper objectMapper) { - this.practiceFindingRepository = practiceFindingRepository; - this.objectMapper = objectMapper; - }As per coding guidelines, "Keep business logic in services annotated with
@Service", "Use constructor injection via@RequiredArgsConstructorannotation", and "Use@Slf4jlogging with parameterized log messages instead ofSystem.out.println()."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProvider.java` around lines 28 - 41, Annotate the ContributorHistoryProvider class with `@Service`, `@RequiredArgsConstructor`, and `@Slf4j`, remove the manual constructor and the private static Logger log = LoggerFactory... field, and let Lombok generate the constructor and logger; retain the final fields practiceFindingRepository and objectMapper and the MAX_PRACTICES constant, and update any logging calls in the class to use the Lombok-provided "log" instance with parameterized messages. Ensure the Lombok annotations are imported (lombok.RequiredArgsConstructor, lombok.extern.slf4j.Slf4j) and the Spring `@Service` annotation is imported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandler.java`:
- Around line 643-661: The verifyActivePractices(AgentJob) method currently
performs the same practiceRepository.findByWorkspaceIdAndActiveTrue(workspaceId)
query that buildPrompt(...) performs later; avoid the duplicate DB call by
having verifyActivePractices return the List<Practice> (or store it on the
AgentJob/prep context) and pass that List into buildPrompt so buildPrompt uses
the already-fetched practices instead of re-querying. Update
verifyActivePractices signature to return List<Practice> (or set a field on
AgentJob), adjust callers in PullRequestReviewHandler to accept the returned
practices (or read from the job/context), and remove the redundant
findByWorkspaceIdAndActiveTrue call from buildPrompt.
In
`@server/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProvider.java`:
- Around line 28-41: Annotate the ContributorHistoryProvider class with
`@Service`, `@RequiredArgsConstructor`, and `@Slf4j`, remove the manual constructor
and the private static Logger log = LoggerFactory... field, and let Lombok
generate the constructor and logger; retain the final fields
practiceFindingRepository and objectMapper and the MAX_PRACTICES constant, and
update any logging calls in the class to use the Lombok-provided "log" instance
with parameterized messages. Ensure the Lombok annotations are imported
(lombok.RequiredArgsConstructor, lombok.extern.slf4j.Slf4j) and the Spring
`@Service` annotation is imported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb966380-5a8b-49e1-a64d-38c27fd6faab
📒 Files selected for processing (9)
server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/JobTypeHandlerConfiguration.javaserver/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandler.javaserver/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProvider.javaserver/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/ContributorPracticeSummary.javaserver/application-server/src/main/java/de/tum/in/www1/hephaestus/practices/finding/PracticeFindingRepository.javaserver/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/handler/JobTypeHandlerRegistryTest.javaserver/application-server/src/test/java/de/tum/in/www1/hephaestus/agent/handler/PullRequestReviewHandlerTest.javaserver/application-server/src/test/java/de/tum/in/www1/hephaestus/practices/finding/ContributorHistoryProviderTest.javaserver/application-server/src/test/java/de/tum/in/www1/hephaestus/practices/finding/PracticeFindingRepositoryIntegrationTest.java
Contributor
📚 Documentation Preview
|
Collaborator
Author
|
🎉 This PR is included in version 0.51.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
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
Closes #895
The practice-aware review agent receives PR metadata, diffs, comments, and practice definitions — but zero contributor history. Without it, the Cognitive Apprenticeship (CA) progression (MODELING → COACHING → SCAFFOLDING → ARTICULATION) is fiction:
guidanceMethodis guessed in isolation every time.This PR injects an aggregated summary (not raw findings) of each contributor's practice verdict history into the agent sandbox as
.context/contributor_history.json, giving the agent the signal it needs to calibrate guidance.What changed
New files (2 production, 1 test):
ContributorPracticeSummary.java— Spring Data projection interface for the JPQL aggregate query. Returns(practiceSlug, verdict, count, lastDetectedAt)per group.ContributorHistoryProvider.java— Stateless transformer that queries the repository, filters outNOT_APPLICABLEverdicts, aggregates by practice, sorts by NEGATIVE count desc, caps at 20 practices, and serializes to compact JSONbyte[]. ReturnsOptional.empty()on no data or serialization failure (graceful degradation).ContributorHistoryProviderTest.java— 9 unit tests covering: empty findings, all-NOT_APPLICABLE, single/multi practice aggregation, NOT_APPLICABLE exclusion, 20-practice cap with sort order, alphabetical tiebreaker, lastSeen max across verdicts, and JsonProcessingException catch path.Modified files (6):
PracticeFindingRepository.java— AddedfindContributorPracticeSummary()JPQL query withGROUP BY slug, verdictandORDER BY slug, verdict. Uses existingidx_practice_finding_contributor_detectedindex.PullRequestReviewHandler.java— Loads PR once inprepareInputFiles()and threads to bothstoreMetadataAndComments()and newstoreContributorHistory(). History injection is wrapped in try-catch for graceful degradation (DB failure doesn't crash the job). Prompt updated with CA method calibration instructions.JobTypeHandlerConfiguration.java— AddedPracticeFindingRepositoryas constructor field, newcontributorHistoryProvider()@bean (package-private), wired into handler via CGLIB proxy call to stay within the 6-parameter arch test limit.PullRequestReviewHandlerTest.java— 4 new tests: history present, history empty, no author skip, provider exception graceful degradation. Addedverify()calls on existing tests.PracticeFindingRepositoryIntegrationTest.java— 7 new integration tests: empty results, single/multi practice aggregation, workspace isolation, correct MAX(detectedAt), contributor isolation, NOT_APPLICABLE verdict inclusion.JobTypeHandlerRegistryTest.java— Updated constructor call for new 9th parameter.Design decisions
[{practice, positive, negative, needsReview, lastSeen}]idx_practice_finding_contributor_detectedindex sufficient for the GROUP BY query.JSON output format
[ {"practice":"pr-description-quality","positive":1,"negative":3,"needsReview":0,"lastSeen":"2026-03-20T14:30:00Z"}, {"practice":"commit-message-quality","positive":2,"negative":0,"needsReview":0,"lastSeen":"2026-03-18T10:15:00Z"} ]What this does NOT include
How to Test
cd server/application-server && ./mvnw test -Dsurefire.includedGroups="unit" -Dmaven.test.skip=false -T 2C --batch-mode -qcd server/application-server && ./mvnw test -Dsurefire.includedGroups="architecture" -Dmaven.test.skip=false -T 2C --batch-mode -qcd server/application-server && ./mvnw test -Dsurefire.includedGroups="integration" -Dmaven.test.skip=false(needs PostgreSQL)contributor_history.jsonappears in agent sandbox.context/directory during a real PR reviewguidanceMethodfor repeat offenders vs first-timersSummary by CodeRabbit
Release Notes
New Features
Tests