Skip to content

AI subsystem: review fixes#21144

Merged
TurboGit merged 30 commits into
darktable-org:masterfrom
andriiryzhkov:review_fix
May 26, 2026
Merged

AI subsystem: review fixes#21144
TurboGit merged 30 commits into
darktable-org:masterfrom
andriiryzhkov:review_fix

Conversation

@andriiryzhkov
Copy link
Copy Markdown
Collaborator

@andriiryzhkov andriiryzhkov commented May 26, 2026

I ran a multi-pass AI-assisted review over the new AI subsystem code – headers, core implementation, domain logic, UI layer, Lua bindings, and a few adjacent non-AI files. I traced every claim line-by-line against the actual code before touching anything, and roughly one in five survived that check. The rest is in this PR.

Everything here is small and preventive. No new features, no behaviour rewrites, no refactors beyond the minimum needed for a specific bug. Each finding is its own commit so anything can be cherry-picked or reverted on its own.

What's in here

  • NULL-deref guards where I was dereferencing helper return values without checking – the safe pattern already existed elsewhere in the same files, the new code just hadn't picked it up.
  • Cleanup on error paths: mipmap cache release, stale raw-preview crops on image switch, partial DNG/TIFF files unlinked when the write fails partway.
  • Atomic disk writes via temp + rename (.dtmodel install, segmentation disk cache). A crash mid-write no longer leaves a half-written file that confuses the next run.
  • Surfacing return codes that were getting silently swallowed – dt_ai_run, TIFFWriteScanline, decoder warmup.
  • Bounds and sanity checks for caller-controlled inputs: densecrf sigmas, decoder shape updates that would otherwise read past the mask buffer.
  • Runtime preflights so ORT can't abort() the process during execution-provider init on a broken CUDA or ROCm stack.
  • API cleanup: dt_ai_registry_t is now opaque to external callers, the unused ep_flags parameter is gone, dt_ai_snapshot_conf_state is idempotent.
  • Documentation: threading contracts and pointer-lifetime contracts where they weren't written down.

Risk

Low. Most fixes are one-line guards or unlink-on-failure additions. The largest single change is the opaque-registry refactor (f4bd660574) – a mechanical rename that keeps the same semantics under the same lock as before.

@andriiryzhkov andriiryzhkov added this to the 5.6 milestone May 26, 2026
@andriiryzhkov andriiryzhkov added bugfix pull request fixing a bug scope: AI features AI features related issues and PR labels May 26, 2026
Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Some style comments, and a better way to create a temp file.

Comment thread src/common/ai/segmentation.c Outdated
Comment thread src/common/ai_models.c Outdated
Comment thread src/common/ai_models.c Outdated
Comment thread src/common/ai_models.h Outdated
Comment thread src/common/ai_models.h Outdated
Comment thread src/ai/backend_common.c Outdated
Comment thread src/ai/backend_common.c Outdated
Comment thread src/ai/backend.h Outdated
Comment thread src/ai/backend.h Outdated
Comment thread src/ai/backend.h Outdated
@andriiryzhkov
Copy link
Copy Markdown
Collaborator Author

@TurboGit : Fixed according to the comments

@andriiryzhkov andriiryzhkov requested a review from TurboGit May 26, 2026 20:08
Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Thanks!

@TurboGit TurboGit merged commit eae9b38 into darktable-org:master May 26, 2026
5 checks passed
@andriiryzhkov andriiryzhkov deleted the review_fix branch May 27, 2026 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug scope: AI features AI features related issues and PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants