feat(x2a): adversarial agents - #4190
Conversation
Changed Packages
|
PR Summary by Qodofeat(x2a): add adversarial agents CRUD, project selection, and review job output
AI Description
Diagram
High-Level Assessment
Files changed (86)
|
Code Review by Qodo
1.
|
rhdh-qodo-merge
Bot
added
documentation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4190 +/- ##
==========================================
- Coverage 58.30% 58.27% -0.03%
==========================================
Files 2427 2438 +11
Lines 96721 97345 +624
Branches 26915 27111 +196
==========================================
+ Hits 56390 56727 +337
- Misses 38869 39154 +285
- Partials 1462 1464 +2
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
mareklibra
left a comment
mareklibra
left a comment
There was a problem hiding this comment.
Important: add AdversarialAgentsPickerExtension to plugins/x2a/app-config.yaml
Please have a look at the SonarQube as well, some of the findings make sense.
| moduleName: module.name, | ||
| sourceRepo: targetRepo, | ||
| targetRepo, | ||
| adversarialAgents, |
There was a problem hiding this comment.
Shouldn't this list be filtered per phase (migrate, analyze)?
There was a problem hiding this comment.
added filter of agents by phase
| onCancelPhase={onCancelPhase} | ||
| /> | ||
| )} | ||
| {onRunAdversarial && |
There was a problem hiding this comment.
Hide/disable the button when the project has no adversarial agent snapshots.
Should the user get info that there are no agents?
There was a problem hiding this comment.
Disable "Run Adversarial Review" button when no agents configured
| }, | ||
| })); | ||
|
|
||
| export const AdversarialJobDetails = ({ |
There was a problem hiding this comment.
AdversarialJobDetails shows status/logs/report but has no cancel control and /modules/.../cancel still only accepts Phase.modulePhaseValues() (analyze/migrate/publish).
A stuck adversarial job can only be cleared via reconcile/409 workarounds. Please either extend cancel to adversarial phases or document that cancellation is unsupported for now.
There was a problem hiding this comment.
Extended cancel to adversarial phases using CancellablePhase schema
| ruleIds: requestBody.acceptedRuleIds ?? [], | ||
| }); | ||
|
|
||
| // Attach adversarial agents if provided (validates agent IDs exist) |
There was a problem hiding this comment.
Same pattern as rules attach: project is persisted first; invalid adversarialAgentIds then throws and leaves an orphan project despite a failed create response.
Prefer pre-validating IDs before createProject.
Don't use transactions, this could get complicated with non-PSQL DBs.
There was a problem hiding this comment.
added validation of rule and agent IDs before createProject to avoid orphan projects
| required: | ||
| - phase | ||
| - moduleId | ||
| - targetRepoAuth |
There was a problem hiding this comment.
OpenAPI marks targetRepoAuth required on /adversarial-run, but the Zod schema + resolveTargetOnly allow omitting it when x2a.git.targetRepo.token is configured. Align the spec with runtime (make it optional) so generated clients and docs match behavior.
| const id = crypto.randomUUID(); | ||
| const now = new Date(); | ||
|
|
||
| await this.#dbClient('adversarial_agents').insert({ |
There was a problem hiding this comment.
Route Zod now constrains phases/name/prompt, but createAdversarialAgent still inserts before constructing AdversarialAgentEntity. Prefer construct/validate first so a future validation drift cannot leave orphan rows.
| ); | ||
|
|
||
| // Attach attempt stats per phase | ||
| const phases = ['analyze', 'migrate', 'publish'] as const; |
There was a problem hiding this comment.
Would adversarial make sense here?
There was a problem hiding this comment.
It would, Extended attempt stats to include adversarial phases
| .notNullable() | ||
| .defaultTo('init') | ||
| .checkIn(['init', 'analyze', 'migrate', 'publish']); | ||
| .checkIn([ |
There was a problem hiding this comment.
Existing migration schemas should not be updated. Please add a new one. The change would be on a fresh-install only.
There was a problem hiding this comment.
Reverted existing migration, handle constraint in the new migration instead
|
| // Step 2: Create the Kubernetes job | ||
| const job = JobResourceBuilder.buildJobSpec(params, this.#config); | ||
| // The adversarial agents ConfigMap is only relevant for adversarial phases | ||
| const isAdversarialPhase = params.phase.startsWith('adversarial-'); |
There was a problem hiding this comment.
what about if we use the src/domain/Phase.ts and load from the string and do some kind of
Phase.isAdversarial()
So it's clear, and more dynamic for the future use cases.
| /> | ||
| )} | ||
| {onRunAdversarial && | ||
| (phaseName === 'analyze' || phaseName === 'migrate') && |
There was a problem hiding this comment.
could we use the Domain object here and do phase.IsAnalyze() or phase.IsMigrate(), or (phase.HasAdversarial()) instead of raw string comparations?
| )} | ||
| {onRunAdversarial && | ||
| (phaseName === 'analyze' || phaseName === 'migrate') && | ||
| phase?.status === 'success' && ( |
There was a problem hiding this comment.
phase.isSuccess() it's a bit better, using the VO object, so not string comparations out there.
| job={adversarialMigratePhase} | ||
| projectId={projectId} | ||
| moduleId={moduleId} | ||
| phaseName="adversarial-migrate" |
There was a problem hiding this comment.
I would skip this string here
3 participants
Adds adversarial agents - a way to configure agents that check the LLM's output during conversion.
Tested on RHDH 1.10.2 as a dynamic plugin . created agents, selected them in the scaffolder, ran a conversion, and verified adversarial job details appear on the module page.