Skip to content

refactor(aireview): move domain models and migrations into plugin - #104

Open
mfrancisc wants to merge 16 commits into
konflux-ci:mainfrom
mfrancisc:aireviewplugin
Open

mfrancisc wants to merge 16 commits into
konflux-ci:mainfrom
mfrancisc:aireviewplugin

Conversation

@mfrancisc

Copy link
Copy Markdown

Problem

Three domain model files (AiReview, AiFailurePrediction, AiPredictionMetrics)
were added to backend/core/models/domainlayer/code/ and registered in core's
GetDomainTablesInfo() and migration scripts. This was incorrect for two reasons:

1. Plugin-specific types in core
The core/models/domainlayer/ package tracks upstream Apache DevLake domain types.
Adding aireview-specific structs here pollutes core with plugin concerns and creates
maintenance overhead on every upstream rebase.

2. Scope deletion registration was non-functional
Registering the tables in GetDomainTablesInfo() implied scope deletion support,
but the aireview converters never populate _raw_data_params (they operate on
project-level aggregations, not raw scope data). The deletion query would silently
match zero rows. The registration was cosmetic.

Solution

  • Removed the 3 domain structs from core/models/domainlayer/code/
  • Removed their entries from GetDomainTablesInfo() and the core migration registry
  • Deleted the 2 core migration scripts
  • Created plugins/aireview/models/domain/ package with the same structs
  • Added an idempotent plugin-owned migration (20260612000001_claim_domain_tables)
    using AutoMigrate — safe for both new installs and existing production databases
    that already have the tables from the old core migrations
  • Updated all convert tasks and their tests to import from the new package

Impact

  • No breaking change for existing deployments
  • Scope deletion for aireview tables was already silently broken — no regression
  • Data cleanup is handled naturally by re-running aireview (converters upsert by ID)
  • Eliminates upstream divergence in domaininfo.go and register.go

Testing

Verified end-to-end on a local podman instance against konflux-ci/mobster
(May–June 2026 data window):

  • ai_reviews: 25 rows
  • ai_failure_predictions: 22 rows
  • ai_prediction_metrics: 8 rows

mfrancisc and others added 2 commits June 12, 2026 19:55
The aireview domain structs (AiReview, AiFailurePrediction,
AiPredictionMetrics) were previously added to core/models/domainlayer/code/
and registered in core's GetDomainTablesInfo() and migration registry.
This violated the architecture rule that core cannot import from plugins
and introduced unnecessary upstream divergence.

This commit:
- Removes the 3 domain struct files from core/models/domainlayer/code/
- Removes their entries from GetDomainTablesInfo() in domaininfo.go
- Deletes the 2 core migration scripts and their register.go entries
- Adds plugins/aireview/models/domain/ package with the 3 structs
- Adds an idempotent plugin migration (20260612000001_claim_domain_tables)
  that uses AutoMigrate to ensure tables exist on both new and existing
  deployments without breaking production instances
- Updates all convert tasks and tests to import from the new package

Verified end-to-end locally: ai_reviews (25), ai_failure_predictions (22),
and ai_prediction_metrics (8) rows written correctly after a pipeline run.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

codecov-commenter commented Jun 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 30.30303% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.70%. Comparing base (ea92013) to head (f114dda).

Files with missing lines Patch % Lines
...ationscripts/20260612000001_claim_domain_tables.go 56.25% 4 Missing and 3 partials ⚠️
...ckend/plugins/aireview/tasks/convert_ai_reviews.go 0.00% 4 Missing ⚠️
...gins/aireview/tasks/convert_failure_predictions.go 0.00% 4 Missing ⚠️
...ugins/aireview/tasks/convert_prediction_metrics.go 0.00% 4 Missing ⚠️
backend/plugins/aireview/impl/impl.go 0.00% 3 Missing ⚠️
backend/plugins/testregistry/impl/impl.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
- Coverage   40.80%   40.70%   -0.11%     
==========================================
  Files         147      151       +4     
  Lines       10189    10219      +30     
==========================================
+ Hits         4158     4160       +2     
- Misses       5927     5955      +28     
  Partials      104      104              
Flag Coverage Δ
e2e-go 9.74% <30.30%> (+0.09%) ⬆️
unit-tests-python 55.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ns/aireview/models/domain/ai_failure_prediction.go 0.00% <ø> (ø)
...ns/aireview/models/domain/ai_prediction_metrics.go 0.00% <ø> (ø)
...ackend/plugins/aireview/models/domain/ai_review.go 0.00% <ø> (ø)
...ugins/aireview/models/migrationscripts/register.go 100.00% <100.00%> (ø)
backend/plugins/testregistry/impl/impl.go 55.26% <0.00%> (-0.49%) ⬇️
backend/plugins/aireview/impl/impl.go 35.50% <0.00%> (-0.65%) ⬇️
...ckend/plugins/aireview/tasks/convert_ai_reviews.go 64.78% <0.00%> (-4.23%) ⬇️
...gins/aireview/tasks/convert_failure_predictions.go 64.28% <0.00%> (-4.29%) ⬇️
...ugins/aireview/tasks/convert_prediction_metrics.go 67.53% <0.00%> (-3.90%) ⬇️
...ationscripts/20260612000001_claim_domain_tables.go 52.63% <56.25%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea92013...f114dda. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…tables migration

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@mfrancisc

Copy link
Copy Markdown
Author

@flacatus @kpiwko pls take a look when you have some time.

@flacatus
flacatus requested a review from kpiwko June 16, 2026 09:49
@flacatus

Copy link
Copy Markdown
Member

/ok-to-test

@mfrancisc
mfrancisc requested a review from a team as a code owner July 8, 2026 12:55
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:56 PM UTC · Completed 1:02 PM UTC
Commit: 14477fe · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review

Findings

Medium

  • [package-naming] backend/plugins/aireview/models/domain/ai_review.go:1 — Package name domain deviates from established plugin conventions. All other plugins use package models for their model files. The subdirectory is a necessary consequence of Go's single-package-per-directory constraint (tool-layer and domain-layer models need separate packages), but no other plugin has this pattern yet.
    Remediation: Consider renaming the package (e.g., domainmodels) or documenting the rationale in an ADR so future plugin authors understand when to use this pattern.

  • [missing-authorization] — Non-trivial architectural refactoring (moving 3 domain models from core to plugin, removing 2 core migrations, modifying core registration files) has no linked issue. While the change reduces upstream divergence (which aligns with AGENTS.md policy), a cross-subsystem refactor of this scope benefits from an issue or ADR documenting the rationale.
    Remediation: Create a linked issue or ADR documenting: (1) why domain models need plugin ownership, (2) impact on other plugins, (3) backward-compatibility strategy.

  • [upstream-divergence-not-tracked] docs/upstream-diffs.md — PR modifies core files (domaininfo.go, register.go, removes 2 core migration scripts) without updating docs/upstream-diffs.md. The modifications are removals of fork-specific lines (reducing divergence toward upstream), but AGENTS.md asks to check/update upstream-diffs.md before modifying files outside owned plugin directories.
    Remediation: Add a brief note to docs/upstream-diffs.md documenting that aireview domain table entries were removed from core (reducing divergence).

Low

  • [migration correctness] backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go — The new claimDomainTables migration consolidates two previously separate core migrations into a single plugin migration with a new version. The old migration history records will become orphaned entries in _devlake_migration_history. Orphaned records have zero runtime impact but will persist indefinitely as dead metadata.

  • [import-alias] backend/plugins/aireview/tasks/convert_ai_reviews.go:27 — The import uses the default package name domain which could hypothetically conflict if another domain package is imported in the same file. Consider using a descriptive alias like aiDomain.

  • [scope-creep] backend/plugins/table_info_test.go — PR bundles opportunistic changes: table_info_test.go adds 3 unrelated plugin registrations (agentready, testregistry, codecov); testregistry adds TektonTask model; api.go has whitespace-only alignment. These are low-risk additions that increase test coverage but extend beyond the stated refactoring scope.

  • [outdated-directory-structure] backend/plugins/aireview/README.md — The README's Directory Structure section lists model files under models/ without mentioning the new models/domain/ subdirectory created by this refactoring.

Previous run

Review

Findings

Medium

  • [stale-doc] AGENTS.md — AGENTS.md states domain models are in backend/core/models/domainlayer/, but this PR moves AiReview, AiFailurePrediction, and AiPredictionMetrics to plugins/aireview/models/domain/. This establishes a new pattern where plugin-specific domain models live within the plugin rather than in core, which is not yet reflected in the documentation.
    Remediation: Update AGENTS.md to clarify that plugin-specific domain models may live within their plugin directory rather than in core.

Low

  • [package-naming] backend/plugins/aireview/models/domain/ai_review.go — The domain sub-package is novel within the plugins directory. Other plugins place all models directly in models/, though sub-directories under models/ exist in a few plugins (e.g., opsgenie/models/raw/). The functional motivation (separating domain-layer models from tool-layer models within the same plugin, avoiding filename collisions) is valid. No functional impact.

  • [import-alias] backend/plugins/aireview/tasks/convert_ai_reviews.go — The old code used the aliased import domainCode for the generic code package. The new unaliased domain import is cleaner since the package name is self-descriptive. This is an improvement over the previous pattern.

  • [stale-doc] backend/DevelopmentManual.md — References core/models/domainlayer as the complete list of domain models. This is an upstream Apache DevLake reference that was already incomplete for fork-specific models — low impact.

  • [scope-creep] backend/server/api/api.go — Whitespace-only change (tab alignment in struct literal) unrelated to the aireview refactor. Harmless but adds noise to the diff.


Labels: Review identified documentation staleness in AGENTS.md related to domain model location

Previous run (2)

Review

Findings

Medium

  • [missing-authorization] — Non-trivial architectural refactor (19 files changed, domain model relocation, migration deletions) has no linked issue. The PR body provides thorough documentation of the problem and solution, but per project conventions, changes of this scope benefit from a linked issue for traceability.
    Remediation: Create an issue documenting the problem, proposed solution, and impact analysis. Link the issue to this PR.

  • [stale-architecture-documentation] AGENTS.md:23 — AGENTS.md states "Domain (standardized tables in backend/core/models/domainlayer/)" but this PR establishes a pattern where plugins can own domain models in a plugin-local models/domain/ package. This architectural change is not reflected in documentation.
    Remediation: Update AGENTS.md to clarify that while most domain models live in backend/core/models/domainlayer/, plugin-specific domain models may be owned by their respective plugins.

Low

  • [behavioral-change] backend/core/models/domainlayer/domaininfo/domaininfo.go:33 — Removing AI domain models from GetDomainTablesInfo() changes /plugininfo REST endpoint output. Scope cleanup runtime behavior is preserved since the plugin's GetTablesInfo() now includes these tables and converters never populated _raw_data_params.

  • [package-organization] backend/plugins/aireview/models/domain/ — Creating a models/domain/ subdirectory is uncommon for plugins but not unprecedented — other plugins use models/raw/ subdirectories. The separation clearly distinguishes domain-layer from tool-layer models.

  • [scope-creep] backend/plugins/table_info_test.go — Adds 4 plugin entries (agentready, aireview, testregistry, codecov) beyond the stated scope. These fix pre-existing test gaps required for CI to pass with ValidatePluginCount: true.

  • [scope-creep] backend/plugins/testregistry/impl/impl.go — Adds TektonTask to GetTablesInfo() — a pre-existing missing registration, not related to aireview refactor but required for the test checker.

  • [migration-hygiene] backend/core/models/migrationscripts/register.go:148 — Removing two migration entries leaves orphan rows in _devlake_migration_history. The migrator ignores unregistered history entries, so this is harmless.

  • [architectural-coherence] backend/core/models/domainlayer/domaininfo/domaininfo.go — The PR body's claim that scope deletion was non-functional is substantiated: converters never populate _raw_data_params, so the deletion query would match zero rows. Verify no Grafana dashboards query these tables via the domain info path.

  • [stale-doc] AGENTS.md — The common pitfall about GetTablesInfo() and the Development Manual's domain model guidance could be updated to mention the plugin-owned domain model pattern.

Previous run (3)

Review

Findings

Medium

  • [architectural-coherence] backend/core/models/domainlayer/domaininfo/domaininfo.go:30 — Removing AI review domain models from GetDomainTablesInfo() changes the established pattern where domain tables are registered centrally in core. The tables remain discoverable via the plugin's GetTablesInfo() (added in impl.go lines 90–92), and no other plugin currently imports or consumes these tables, so the practical impact is limited. However, this establishes a precedent for plugin-owned domain tables that diverges from the upstream DevLake architecture. Consider documenting this decision in an ADR.

Low

  • [schema-drift] backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go — The frozen migration struct declares UpdatedAt *time.Time (nullable) while the runtime DomainEntity via NoPKModel uses UpdatedAt time.Time (non-nullable). This is inherited from the previous migration and has no practical MySQL impact, but creates a minor nullability inconsistency between the migration schema and the runtime model.

  • [scope-creep] backend/server/api/api.go — Includes whitespace-only reformatting of the CORS configuration (aligning struct field assignments) that is unrelated to the aireview domain model refactoring described in the PR.

  • [consistency] backend/plugins/table_info_test.go — Adds checker.FeedIn calls for agentready, aireview, testregistry, and codecov plugins, and adds TektonTask to testregistry's GetTablesInfo(). The PR description only mentions aireview changes — these appear to be pre-existing test registration gaps being fixed opportunistically.

Previous run (4)

Review

Findings

Medium

  • [architectural-coherence] backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go — The migration script creates domain tables with version 20260612000001, replacing core migration scripts at earlier versions (20260422000001, 20260422000002). Existing installations that already ran the core migrations will additionally run the new plugin migration; fresh installations will run only the plugin migration. AutoMigrate is idempotent so both paths converge on the same schema, but the migration history will differ between fresh and upgraded installations. A brief code comment explaining the idempotent upgrade strategy would help future maintainers.
    Remediation: Add a comment to the migration's Up() method noting that AutoMigrate handles both fresh installs (creates tables) and upgrades from the old core migrations (no-ops on existing columns).

Low

  • [missing-authorization] No linked issue found for this PR. The PR performs a structural refactoring (moving domain models from core to plugin ownership) across 20 files. Consider creating a tracking issue documenting the decision.

  • [upstream-divergence-undocumented] docs/upstream-diffs.md — The new table_info_test entry correctly documents owned plugin registrations with all required fields. However, the domain model move itself — removing AiReview, AiFailurePrediction, AiPredictionMetrics from core/models/domainlayer/code/ and the two core migration scripts — is a separate modification to upstream-tracked files that may warrant its own entry in upstream-diffs.md.

Previous run (5)

Review

Findings

Medium

  • [upstream-divergence-undocumented] — This PR modifies files outside the owned plugin directory (removes entries from core/models/domainlayer/domaininfo/domaininfo.go and core/models/migrationscripts/register.go) but does not update docs/upstream-diffs.md. Per AGENTS.md, all modifications to files originating from upstream Apache DevLake must be tracked there.
    Remediation: Add an entry to docs/upstream-diffs.md documenting the divergence: files removed from core, reason, owner, and rebase notes.

Low

  • [consumer-completeness] backend/plugins/aireview/impl/impl.go:84 — The three domain model types were removed from core's GetDomainTablesInfo() but not added to the plugin's GetTablesInfo(). While scope deletion was already non-functional for these tables (converters never populate RawDataParams and the plugin has no scope service), adding them to GetTablesInfo() would improve completeness and future-proofing.

  • [architecture-coherence] — Moves domain models from core/models/domainlayer/code/ to plugins/aireview/models/domain/, diverging from the documented three-layer architecture. These are Konflux-specific additions not present in upstream Apache DevLake, making the encapsulation reasonable but worth documenting.

  • [missing-authorization] — Non-trivial architectural refactor with no linked issue. The PR body provides clear rationale, but linking a JIRA issue would improve traceability.

  • [edge-case] backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go:133 — Orphaned migration history entries from old core migrations (20260422000001, 20260422000002) persist in _devlake_migration_history. The framework tolerates this but it could cause confusion during debugging.

Previous run (6)

Review — refactor(aireview): move domain models and migrations into plugin

Verdict: approve · No blocking findings.

Summary

Clean, well-scoped refactoring that moves three plugin-specific domain models (AiReview, AiFailurePrediction, AiPredictionMetrics) out of the shared core domain layer and into the aireview plugin. This eliminates upstream divergence in domaininfo.go and register.go, and consolidates two core migration scripts into one idempotent plugin-owned migration.

What was verified

Correctness & migration safety:

  • All 9 references to domainCode.AiReview, domainCode.AiFailurePrediction, and domainCode.AiPredictionMetrics across converters and tests are updated to the new domain package import.
  • The new plugin migration (20260612000001_claim_domain_tables) uses AutoMigrate, which is idempotent — safe for both new installs (creates tables) and existing databases (no-op since tables already exist from old core migrations).
  • The frozen migration structs include _raw_data_* columns inline, correctly merging the work of both removed core migrations (20260422000001 + 20260422000002) into a single migration.
  • The domain struct definitions are identical to the originals (only the package declaration changes from code to domain).
  • Removal from GetDomainTablesInfo() is correct — scope deletion for these tables was already non-functional since converters never populate _raw_data_params.

Security: No new attack surface, secrets, or privilege changes.

Intent & scope: The change matches the stated goal of reducing upstream divergence. All modifications are coherent with the architectural direction of keeping plugin-specific types in the plugin.

Minor observations

  1. backend/server/api/api.go — The whitespace-alignment change (lines 106–108) is cosmetic and modifies an upstream-origin file. It may cause a trivial merge conflict on the next upstream rebase. Not blocking, but consider reverting to keep the upstream file untouched.

  2. backend/plugins/table_info_test.go — The import reorder (circleci/claudeCode) is also cosmetic. Same minor rebase friction concern.

Neither observation affects correctness or warrants changes-requested status.

Previous run (7)

Review — approve

Clean, well-scoped refactoring that moves plugin-specific domain models and migrations out of core into the aireview plugin. The change correctly reduces upstream divergence in domaininfo.go and register.go.

What was verified

Correctness

  • The three domain structs (AiReview, AiFailurePrediction, AiPredictionMetrics) are moved intact — only the package declaration changes from code to domain. All fields and GORM tags are preserved.
  • The new consolidated migration (20260612000001) correctly merges the old table creation (20260422000001) and column fix (20260422000002) into a single idempotent AutoMigrate with _raw_data_* fields inline in the frozen structs.
  • Migration safety for existing deployments: the old core migration entries remain in _devlake_migration_history (tracked by ScriptVersion + ScriptName) but are no longer registered, so they are inert. The new plugin migration has a distinct version (20260612000001) and name, so it runs independently. AutoMigrate is a no-op when tables already exist with the correct schema.
  • Migration safety for new deployments: the aireview plugin migration creates the tables from scratch since core no longer does.
  • All import paths are updated consistently across the 3 converters, 3 unit test files, and the e2e test file.
  • The claimDomainTables registration in plugins/aireview/models/migrationscripts/register.go is appended at the end, preserving expected migration ordering.

Security — No security-relevant changes. Pure structural refactoring.

Scope & intent — The diff matches the stated purpose. The removal from GetDomainTablesInfo() is correctly justified: the aireview converters never populate _raw_data_params, so scope deletion was already non-functional.

Style — Package name domain is appropriate. Import ordering follows goimports conventions. The two cosmetic changes (api.go CORS alignment, table_info_test.go import reorder) are minor formatting fixes that happen to be in the same branch.

Documentation — docs/upstream-diffs.md does not need updating: the core files (domaininfo.go, register.go) were not explicitly tracked there, and this PR removes rather than adds upstream divergence.

Previous run (8)

Review — approve

Clean refactoring that correctly moves plugin-specific domain models out of core and into the aireview plugin, reducing upstream divergence and improving encapsulation.

What was verified

Correctness

  • All 3 domain model files (AiReview, AiFailurePrediction, AiPredictionMetrics) are renamed from core/models/domainlayer/code/ to plugins/aireview/models/domain/ with only the package declaration changing — struct definitions, table names, and GORM tags are preserved exactly.
  • All import references across 6 converter/test files and the e2e test are consistently updated from domainCode ".../code" to ".../models/domain".
  • No other consumers of code.AiReview, code.AiFailurePrediction, or code.AiPredictionMetrics exist outside the files touched by this PR — grep confirmed zero unupdated references.
  • Entries removed from GetDomainTablesInfo() and the core migration register.go; the code import stays in domaininfo.go since it still references other types (Commit, PullRequest, etc.).

Migration safety

  • The new plugin-owned migration (20260612000001_claim_domain_tables) uses AutoMigrate, which is idempotent — on existing deployments where core migrations already created the tables, it becomes a no-op (or safely adds missing columns).
  • The frozen struct snapshots in the migration correctly include _raw_data_* columns, consolidating what previously required two separate core migrations (create tables + add columns) into a single step.
  • Removing old migrations from core's All() is safe: the DevLake migration runner records completed versions in _devlake_migration_history and does not validate that all recorded versions remain registered.
  • Version 20260612000001 correctly sorts after the latest existing plugin migration (20260415000001).

Upstream divergence

  • This PR removes aireview-specific entries from two core files (domaininfo.go, register.go) and deletes two core migration scripts, bringing those files closer to upstream Apache DevLake. This is a net reduction in fork maintenance overhead, consistent with AGENTS.md guidance that owned plugins should be self-contained additions.

Incidental changes

  • table_info_test.go: import reorder (circleci before claudeCode) — cosmetic, likely from goimports.
  • api.go: CORS config struct field alignment — cosmetic whitespace normalization.

No security, correctness, or architectural concerns identified.


Labels: Refactoring that improves plugin architecture

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge enhancement New feature or request labels Jul 8, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 14, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:56 AM UTC · Completed 8:02 AM UTC
Commit: 14477fe · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jul 14, 2026
Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 14, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:26 AM UTC · Completed 8:32 AM UTC
Commit: 14477fe · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jul 14, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 2:02 PM UTC · Ended 2:17 PM UTC
Commit: 701e62a · View workflow run →

@qodo-app-for-konflux-ci

qodo-app-for-konflux-ci Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📜 Skill insights (0)

Grey Divider


Action required

1. domain.AiReview absent in GetTablesInfo ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The converters now write to plugins/aireview/models/domain tables (e.g., domain.AiReview), but
AiReview.GetTablesInfo() does not list these domain models. This can break table
enumeration/initialization that depends on GetTablesInfo().
Code

backend/plugins/aireview/tasks/convert_ai_reviews.go[R57-58]

+	if err := db.Delete(&domain.AiReview{}, dal.Where("project_name = ?", projectName)); err != nil {
		return errors.Default.Wrap(err, "failed to delete existing ai_reviews for project")
Relevance

●●● Strong

GetTablesInfo completeness checks are typically enforced; missing domain tablers likely breaks
init/enumeration.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1106 requires every model struct used by the plugin to be present in
impl/impl.go:GetTablesInfo(). The PR updates converters to use domain.AiReview, but
GetTablesInfo() still only returns models.* and does not include any domain.* tablers.

Rule 1106: All models must be listed in GetTablesInfo() in impl/impl.go
backend/plugins/aireview/tasks/convert_ai_reviews.go[56-59]
backend/plugins/aireview/impl/impl.go[84-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GetTablesInfo()` for the `aireview` plugin does not include the domain models that the plugin now writes (`domain.AiReview`, `domain.AiFailurePrediction`, `domain.AiPredictionMetrics`).

## Issue Context
The refactor moved these domain structs into `backend/plugins/aireview/models/domain` and updated converters to use them, but `backend/plugins/aireview/impl/impl.go` still only returns tool-layer `models.*` tables.

## Fix Focus Areas
- backend/plugins/aireview/impl/impl.go[84-92]
- backend/plugins/aireview/tasks/convert_ai_reviews.go[56-59]
- backend/plugins/aireview/tasks/convert_failure_predictions.go[49-53]
- backend/plugins/aireview/tasks/convert_prediction_metrics.go[49-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. backend/core changes violate ownership 📘 Rule violation ⌂ Architecture
Description
This PR modifies upstream core/server files (e.g., backend/core/...) instead of limiting changes
to the owned plugin directories. This violates the rule restricting modifications to
backend/plugins/aireview/, backend/plugins/codecov/, or backend/plugins/testregistry/.
Code

backend/core/models/domainlayer/domaininfo/domaininfo.go[L33-35]

-		&code.AiReview{},
-		&code.AiFailurePrediction{},
-		&code.AiPredictionMetrics{},
Relevance

●● Moderate

Compliance/ownership rule, but no matching historical enforcement precedent found in repo searches.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1105 restricts changes to specific owned plugin directories only. The diff shows
edits in backend/core/... (and other non-owned paths), which are outside the allowed plugin
directories.

Rule 1105: Do not modify upstream code outside owned plugin directories
backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
backend/core/models/migrationscripts/register.go[145-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes files outside the allowed owned plugin directories, violating the repository ownership constraint.

## Issue Context
Compliance requires that all modifications in this PR be confined to `backend/plugins/aireview/`, `backend/plugins/codecov/`, or `backend/plugins/testregistry/`. The diff includes edits/deletions in core/server paths.

## Fix Focus Areas
- backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
- backend/core/models/migrationscripts/register.go[145-148]
- backend/server/api/api.go[99-112]
- backend/plugins/table_info_test.go[20-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Upstream changes not in upstream-diffs 📘 Rule violation § Compliance
Description
Files outside owned plugin directories were modified, but no corresponding entry was added to
docs/upstream-diffs.md. This makes future upstream rebases harder to manage and violates the
upstream-divergence documentation requirement.
Code

backend/core/models/migrationscripts/register.go[L148-149]

-		new(addAiReviewDomainTables),
-		new(fixAiReviewDomainColumns),
Relevance

●●● Strong

Team previously accepted adding upstream-divergence documentation when touching upstream files.

PR-#99

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1360 requires documenting new upstream divergences for modified files outside owned
plugin directories. docs/upstream-diffs.md contains tracked divergences but does not list the
modified core file paths.

Rule 1360: Document new upstream divergences in docs/upstream-diffs.md
docs/upstream-diffs.md[1-31]
backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Modified non-owned (upstream) files must be documented in `docs/upstream-diffs.md` when they are not already tracked there.

## Issue Context
This PR changes core migration/domain registration behavior outside owned plugin directories, but `docs/upstream-diffs.md` has no entry for those files.

## Fix Focus Areas
- docs/upstream-diffs.md[1-67]
- backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
- backend/core/models/migrationscripts/register.go[145-148]
- backend/server/api/api.go[99-112]
- backend/plugins/table_info_test.go[20-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Domain migration schema drift 🐞 Bug ⚙ Maintainability
Description
claimDomainTables snapshots create the ai_* domain tables with _raw_data_params missing the
runtime model’s index tag (and with UpdatedAt nullable via *time.Time), so new installs can
end up with a schema that diverges from domainlayer.DomainEntity/common.NoPKModel. This can
degrade performance for any raw-data-param–filtered operations and makes the migration-created
schema inconsistent with the structs used at runtime.
Code

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[R60-63]

+	RawDataParams string `gorm:"column:_raw_data_params;type:varchar(255)"`
+	RawDataTable  string `gorm:"column:_raw_data_table;type:varchar(255)"`
+	RawDataId     uint64 `gorm:"column:_raw_data_id"`
+	RawDataRemark string `gorm:"column:_raw_data_remark;type:longtext"`
Relevance

●● Moderate

Schema/index/tag drift fix seems reasonable but touches migration semantics; no close historical
pattern found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migration’s archived structs omit the _raw_data_params index and use nullable UpdatedAt,
while the runtime base model embedded into all domain entities specifies an index on
_raw_data_params and uses non-pointer timestamps; this creates a schema mismatch for new
installations that rely on this migration to create the tables.

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[60-66]
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[93-99]
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[135-141]
backend/core/models/common/base.go[51-74]
backend/core/models/domainlayer/domainlayer.go[24-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The plugin migration `claimDomainTables` uses archived structs whose GORM tags don’t match the runtime embedded base model (`domainlayer.DomainEntity` -> `common.NoPKModel` -> `common.RawDataOrigin`). In particular, `_raw_data_params` is created without the `index` tag, and `UpdatedAt` is modeled as `*time.Time` (nullable) instead of `time.Time`.

### Issue Context
The runtime schema expectations come from `common.RawDataOrigin` (which sets an index on `_raw_data_params`) and `common.NoPKModel` (which defines `UpdatedAt time.Time`). Since these tables are created/claimed by the migration, any mismatch here becomes the canonical schema for new installs.

### Fix Focus Areas
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[35-141]
 - Add `;index` to `RawDataParams` tags in **all three** archived structs.
 - Change `UpdatedAt` fields in the archived structs to `time.Time` (non-pointer) to match `common.NoPKModel`.
 - (Optional) Consider mirroring `common.RawDataOrigin` tags exactly for the `_raw_data_*` fields so the migration schema stays aligned over time.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

5. Postgres longtext migration 🐞 Bug ☼ Reliability ⭐ New
Description
claimDomainTables hard-codes type:longtext for _raw_data_remark, which can generate invalid
DDL and fail the migration on Postgres-backed deployments. This affects all three aireview domain
tables created/claimed by this migration.
Code

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[R60-63]

+	RawDataParams string `gorm:"column:_raw_data_params;type:varchar(255)"`
+	RawDataTable  string `gorm:"column:_raw_data_table;type:varchar(255)"`
+	RawDataId     uint64 `gorm:"column:_raw_data_id"`
+	RawDataRemark string `gorm:"column:_raw_data_remark;type:longtext"`
Relevance

● Weak

Prior closely-related Postgres portability migration fixes were rejected; likely won’t change
longtext usage now.

PR-#112
PR-#105

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migration sets _raw_data_remark to longtext via GORM tags and then runs AutoMigrate, so
that type will be used in schema creation/alteration. The repo explicitly supports Postgres
connections, and the canonical raw-data model avoids DB-specific types for RawDataRemark, implying
portability is expected.

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[35-66]
backend/core/runner/db.go[123-159]
backend/core/models/common/base.go[51-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`claimDomainTables` uses `gorm:"type:longtext"` for the `_raw_data_remark` column. `longtext` is MySQL-specific and can cause `AutoMigrate` to fail on Postgres (supported by this repo), preventing the plugin migration from completing.

### Issue Context
- The migration runs via `db.AutoMigrate(...)`, so the struct tag directly drives the emitted DDL.
- The core `RawDataOrigin.RawDataRemark` does **not** force a DB-specific type, letting GORM choose a compatible type per dialect.

### Fix Focus Areas
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[60-65]
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[93-98]
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[135-140]

### Suggested fix
- Replace `type:longtext` with a portable type (e.g., `type:text`) or omit the explicit type and let GORM pick the dialect-appropriate large-text type.
- If you must keep `longtext` on MySQL for compatibility, branch on `db.Dialect()` and apply `ModifyColumnType` only for MySQL after a portable `AutoMigrate` (or otherwise implement dialect-specific DDL).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Migration filename uses full timestamp 📘 Rule violation ⚙ Maintainability
Description
The migration script filename 20260612000001_claim_domain_tables.go does not follow the required
YYYYMMDD_description.go naming convention. Tooling or reviewer expectations based on the naming
pattern may fail or become inconsistent.
Code

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[R159-160]

+func (*claimDomainTables) Version() uint64 {
+	return 20260612000001
Relevance

● Weak

Very similar suggestion (rename full-timestamp migration filename) was previously rejected.

PR-#112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1108 requires migration filenames to match YYYYMMDD_description.go. The migration
is stored as 20260612000001_claim_domain_tables.go, which includes a full timestamp rather than an
8-digit date prefix.

Rule 1108: Migration filenames must follow YYYYMMDD_description.go format
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[159-165]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Migration filenames under `models/migrationscripts/` must follow `YYYYMMDD_description.go`, but this PR uses a 14-digit timestamp prefix.

## Issue Context
Current file name: `backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go`.

## Fix Focus Areas
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[1-165]
- backend/plugins/aireview/models/migrationscripts/register.go[24-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 129 rules

Grey Divider

Tip of the day
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous review results

Review updated until commit a79c891

Results up to commit 52c5b9b ⚖️ Balanced


🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. domain.AiReview absent in GetTablesInfo ✓ Resolved 📘 Rule violation ≡ Correctness
Description
The converters now write to plugins/aireview/models/domain tables (e.g., domain.AiReview), but
AiReview.GetTablesInfo() does not list these domain models. This can break table
enumeration/initialization that depends on GetTablesInfo().
Code

backend/plugins/aireview/tasks/convert_ai_reviews.go[R57-58]

+	if err := db.Delete(&domain.AiReview{}, dal.Where("project_name = ?", projectName)); err != nil {
		return errors.Default.Wrap(err, "failed to delete existing ai_reviews for project")
Relevance

●●● Strong

GetTablesInfo completeness checks are typically enforced; missing domain tablers likely breaks
init/enumeration.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1106 requires every model struct used by the plugin to be present in
impl/impl.go:GetTablesInfo(). The PR updates converters to use domain.AiReview, but
GetTablesInfo() still only returns models.* and does not include any domain.* tablers.

Rule 1106: All models must be listed in GetTablesInfo() in impl/impl.go
backend/plugins/aireview/tasks/convert_ai_reviews.go[56-59]
backend/plugins/aireview/impl/impl.go[84-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GetTablesInfo()` for the `aireview` plugin does not include the domain models that the plugin now writes (`domain.AiReview`, `domain.AiFailurePrediction`, `domain.AiPredictionMetrics`).

## Issue Context
The refactor moved these domain structs into `backend/plugins/aireview/models/domain` and updated converters to use them, but `backend/plugins/aireview/impl/impl.go` still only returns tool-layer `models.*` tables.

## Fix Focus Areas
- backend/plugins/aireview/impl/impl.go[84-92]
- backend/plugins/aireview/tasks/convert_ai_reviews.go[56-59]
- backend/plugins/aireview/tasks/convert_failure_predictions.go[49-53]
- backend/plugins/aireview/tasks/convert_prediction_metrics.go[49-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. backend/core changes violate ownership 📘 Rule violation ⌂ Architecture
Description
This PR modifies upstream core/server files (e.g., backend/core/...) instead of limiting changes
to the owned plugin directories. This violates the rule restricting modifications to
backend/plugins/aireview/, backend/plugins/codecov/, or backend/plugins/testregistry/.
Code

backend/core/models/domainlayer/domaininfo/domaininfo.go[L33-35]

-		&code.AiReview{},
-		&code.AiFailurePrediction{},
-		&code.AiPredictionMetrics{},
Relevance

●● Moderate

Compliance/ownership rule, but no matching historical enforcement precedent found in repo searches.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1105 restricts changes to specific owned plugin directories only. The diff shows
edits in backend/core/... (and other non-owned paths), which are outside the allowed plugin
directories.

Rule 1105: Do not modify upstream code outside owned plugin directories
backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
backend/core/models/migrationscripts/register.go[145-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes files outside the allowed owned plugin directories, violating the repository ownership constraint.

## Issue Context
Compliance requires that all modifications in this PR be confined to `backend/plugins/aireview/`, `backend/plugins/codecov/`, or `backend/plugins/testregistry/`. The diff includes edits/deletions in core/server paths.

## Fix Focus Areas
- backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
- backend/core/models/migrationscripts/register.go[145-148]
- backend/server/api/api.go[99-112]
- backend/plugins/table_info_test.go[20-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
3. Upstream changes not in upstream-diffs 📘 Rule violation § Compliance
Description
Files outside owned plugin directories were modified, but no corresponding entry was added to
docs/upstream-diffs.md. This makes future upstream rebases harder to manage and violates the
upstream-divergence documentation requirement.
Code

backend/core/models/migrationscripts/register.go[L148-149]

-		new(addAiReviewDomainTables),
-		new(fixAiReviewDomainColumns),
Relevance

●●● Strong

Team previously accepted adding upstream-divergence documentation when touching upstream files.

PR-#99

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1360 requires documenting new upstream divergences for modified files outside owned
plugin directories. docs/upstream-diffs.md contains tracked divergences but does not list the
modified core file paths.

Rule 1360: Document new upstream divergences in docs/upstream-diffs.md
docs/upstream-diffs.md[1-31]
backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Modified non-owned (upstream) files must be documented in `docs/upstream-diffs.md` when they are not already tracked there.

## Issue Context
This PR changes core migration/domain registration behavior outside owned plugin directories, but `docs/upstream-diffs.md` has no entry for those files.

## Fix Focus Areas
- docs/upstream-diffs.md[1-67]
- backend/core/models/domainlayer/domaininfo/domaininfo.go[30-36]
- backend/core/models/migrationscripts/register.go[145-148]
- backend/server/api/api.go[99-112]
- backend/plugins/table_info_test.go[20-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Domain migration schema drift 🐞 Bug ⚙ Maintainability
Description
claimDomainTables snapshots create the ai_* domain tables with _raw_data_params missing the
runtime model’s index tag (and with UpdatedAt nullable via *time.Time), so new installs can
end up with a schema that diverges from domainlayer.DomainEntity/common.NoPKModel. This can
degrade performance for any raw-data-param–filtered operations and makes the migration-created
schema inconsistent with the structs used at runtime.
Code

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[R60-63]

+	RawDataParams string `gorm:"column:_raw_data_params;type:varchar(255)"`
+	RawDataTable  string `gorm:"column:_raw_data_table;type:varchar(255)"`
+	RawDataId     uint64 `gorm:"column:_raw_data_id"`
+	RawDataRemark string `gorm:"column:_raw_data_remark;type:longtext"`
Relevance

●● Moderate

Schema/index/tag drift fix seems reasonable but touches migration semantics; no close historical
pattern found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The migration’s archived structs omit the _raw_data_params index and use nullable UpdatedAt,
while the runtime base model embedded into all domain entities specifies an index on
_raw_data_params and uses non-pointer timestamps; this creates a schema mismatch for new
installations that rely on this migration to create the tables.

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[60-66]
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[93-99]
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[135-141]
backend/core/models/common/base.go[51-74]
backend/core/models/domainlayer/domainlayer.go[24-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The plugin migration `claimDomainTables` uses archived structs whose GORM tags don’t match the runtime embedded base model (`domainlayer.DomainEntity` -> `common.NoPKModel` -> `common.RawDataOrigin`). In particular, `_raw_data_params` is created without the `index` tag, and `UpdatedAt` is modeled as `*time.Time` (nullable) instead of `time.Time`.

### Issue Context
The runtime schema expectations come from `common.RawDataOrigin` (which sets an index on `_raw_data_params`) and `common.NoPKModel` (which defines `UpdatedAt time.Time`). Since these tables are created/claimed by the migration, any mismatch here becomes the canonical schema for new installs.

### Fix Focus Areas
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[35-141]
 - Add `;index` to `RawDataParams` tags in **all three** archived structs.
 - Change `UpdatedAt` fields in the archived structs to `time.Time` (non-pointer) to match `common.NoPKModel`.
 - (Optional) Consider mirroring `common.RawDataOrigin` tags exactly for the `_raw_data_*` fields so the migration schema stays aligned over time.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational
5. Migration filename uses full timestamp 📘 Rule violation ⚙ Maintainability
Description
The migration script filename 20260612000001_claim_domain_tables.go does not follow the required
YYYYMMDD_description.go naming convention. Tooling or reviewer expectations based on the naming
pattern may fail or become inconsistent.
Code

backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[R159-160]

+func (*claimDomainTables) Version() uint64 {
+	return 20260612000001
Relevance

● Weak

Very similar suggestion (rename full-timestamp migration filename) was previously rejected.

PR-#112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1108 requires migration filenames to match YYYYMMDD_description.go. The migration
is stored as 20260612000001_claim_domain_tables.go, which includes a full timestamp rather than an
8-digit date prefix.

Rule 1108: Migration filenames must follow YYYYMMDD_description.go format
backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[159-165]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Migration filenames under `models/migrationscripts/` must follow `YYYYMMDD_description.go`, but this PR uses a 14-digit timestamp prefix.

## Issue Context
Current file name: `backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go`.

## Fix Focus Areas
- backend/plugins/aireview/models/migrationscripts/20260612000001_claim_domain_tables.go[1-165]
- backend/plugins/aireview/models/migrationscripts/register.go[24-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:02 PM UTC · Completed 2:17 PM UTC
Commit: 701e62a · View workflow run →

@flacatus

flacatus commented Aug 4, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@rsoaresd rsoaresd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work 🚀 ! Just suggestion regarding test:

bitbucket_server "github.com/apache/incubator-devlake/plugins/bitbucket_server/impl"
claudeCode "github.com/apache/incubator-devlake/plugins/claude_code/impl"
circleci "github.com/apache/incubator-devlake/plugins/circleci/impl"
claudeCode "github.com/apache/incubator-devlake/plugins/claude_code/impl"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These tests will fail. I needed to add the following changes to work:

in backend/plugins/table_info_test.go:

  • add the following imports:
agentready "github.com/apache/incubator-devlake/plugins/agentready/impl"
aireview "github.com/apache/incubator-devlake/plugins/aireview/impl"
codecov "github.com/apache/incubator-devlake/plugins/codecov/impl"
testregistry "github.com/apache/incubator-devlake/plugins/testregistry/impl"
  • add the following checkers:
checker.FeedIn("agentready/models", agentready.AgentReady{}.GetTablesInfo)
checker.FeedIn("aireview/models", aireview.AiReview{}.GetTablesInfo)
checker.FeedIn("testregistry/models", testregistry.TestRegistry{}.GetTablesInfo)
checker.FeedIn("codecov/models", codecov.Codecov{}.GetTablesInfo)

in backend/plugins/aireview/impl/impl.go:

  • add the following import
    "github.com/apache/incubator-devlake/plugins/aireview/models/domain"
  • add the following domains in GetTablesInfo():
&domain.AiReview{},
&domain.AiFailurePrediction{},
&domain.AiPredictionMetrics{},

in backend/plugins/testregistry/impl/impl.go:

  • add the following model in GetTablesInfo():
    &models.TektonTask{},

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

good point! Should be now covered by 3f22bdc 🙏

Test_GetPluginTablesInfo failed because owned plugins were not FeedIn'd
and aireview/testregistry omitted domain/TektonTask models. Document the
table_info_test.go fork divergence.

Upstream-Status: N/A — owned plugins are Konflux-only additions

Co-Authored-By: Cursor <cursoragent@cursor.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 1:47 PM UTC · Ended 2:00 PM UTC
Commit: 701e62a · View workflow run →

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3f22bdc

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:47 PM UTC · Completed 2:00 PM UTC
Commit: 701e62a · View workflow run →

@rsoaresd rsoaresd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀

@kpiwko kpiwko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, looks good!

@kpiwko

kpiwko commented Aug 6, 2026

Copy link
Copy Markdown

/retest

@kpiwko

kpiwko commented Aug 6, 2026

Copy link
Copy Markdown

@mfrancisc can you have a look at failing linters? One is likely commit message - 1a24314, the other is about imports in migrations - I think you have already fixed that in the main?

@mfrancisc

Copy link
Copy Markdown
Author

The fix for the migration-script-lint is in this PR: #131 ( it's failing in every PR now )

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 1:58 PM UTC · Ended 2:00 PM UTC
Commit: 701e62a · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 2:01 PM UTC · Ended 2:13 PM UTC
Commit: 701e62a · View workflow run →

@mfrancisc

Copy link
Copy Markdown
Author

@kpiwko commit message linter is fixed now, even if the required rule it's a bit too strict IMHO and I don't really understand why.

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 1f6bacc

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:01 PM UTC · Completed 2:13 PM UTC
Commit: 701e62a · View workflow run →

@qodo-app-for-konflux-ci

qodo-app-for-konflux-ci Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

No code changes since the last review — review skipped

Qodo Logo

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:37 AM UTC · Completed 10:53 AM UTC

Commit: 701e62a · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 10:31 AM UTC · Completed 10:47 AM UTC

Commit: 9ee3c25 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added the documentation Improvements or additions to documentation label Aug 19, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:40 PM UTC · Completed 12:56 PM UTC

Commit: 9ee3c25 · View workflow run →

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

Labels

documentation Improvements or additions to documentation enhancement New feature or request requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants