[FIX] Simplify ORGDIR check for consistent rebuild behavior

[prenetic]

The original heuristic was no longer a real "did the user delete ORGDIR?" check, it was an assumption that only worked when AZ_DIR=True. With AZ_DIR=False those directories are never created, so deleting ORGDIR would go undetected and leave the user without an ORGDIR entirely.

Instead we should just check whether ORGDIR exists. Configuration changes already trigger a rebuild via INI ctime, so this only needs to handle the manual deletion case. This also aligns with the NOTE: Remove the Organized folders if you wish to start from scratch. message that is currently displayed to the user during incremental builds.

Fixes #146 (follow-up to merged AZ_DIR=False fix)

Manual testing performed:

1. A fresh install creates ORGDIR with A-Z subdirectories on first run.
2. Subsequent runs are incremental builds.
3. Removing the ORGDIR directory triggers a full build.
4. Disabling A-Z subdirectories triggers an full build via INI change detection.
5. Subsequent runs are incremental builds.
6. Removing the ORGDIR directory triggers a full build.

Summary by CodeRabbit

• Bug Fixes
  • Improved rebuild detection so the app now only performs a full rebuild when the organized output folder is actually missing.
  • Incremental runs now avoid unnecessary cleanup and rebuild steps when existing organized files are still available.
  • If the organized output folder is deleted, the next run correctly rebuilds everything from scratch.
  • Updated the full-build message wording for clearer status feedback.

[coderabbitai]

📝 Walkthrough

Walkthrough

Infrastructure.check_if_orgdir_directories_are_missing() is replaced with check_if_orgdir_directory_is_missing(), which checks only the top-level ORGDIR directory instead of multiple ORGDIR_* subdirectories. The service call site and console messages are updated accordingly. Integration tests are revised to cover incremental runs with AZ_DIR=True and a new scenario where orgdir is deleted before a second run.

ORGDIR Rebuild Detection

Layer / File(s)	Summary
Infrastructure method and service call src/update_all/arcade_organizer/arcade_organizer.py	check_if_orgdir_directories_are_missing() replaced by check_if_orgdir_directory_is_missing() (single top-level existence check); service conditional, printed message, and "start from scratch" note string updated.
Integration tests src/test/integration/test_arcade_organizer.py	Incremental-run test renamed and reconfigured with AZ_DIR=True, assertion changed to call_count == 0; new test deletes orgdir after first run and verifies rebuild produces matching .mra paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• theypsilon/Update_All_MiSTer#176: Directly modifies the same ORGDIR missing-detection logic and incremental-run integration tests, making it an immediate predecessor to this change.

Poem

🐇 One folder to check, not a maze of subdirs,
The rabbit hops past all the alphabetic stirs.
Delete _Organized? Fear not, it rebuilds anew!
Incremental stays calm when ORGDIR is in view.
Simple existence — that's all that we need! 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The change directly addresses #146 by basing rebuilds on ORGDIR existence, which prevents false full rebuilds when AZ_DIR is disabled.
Out of Scope Changes check	✅ Passed	The only notable additions are targeted tests and a message tweak, both supporting the ORGDIR rebuild fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately summarizes the main change: simplifying ORGDIR detection to make rebuild behavior consistent.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/test/integration/test_arcade_organizer.py (1)

710-716: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Incremental regression test no longer covers the AZ_DIR=False scenario from #146.

Issue #146 reproduces specifically with alphabetic folders disabled — the old multi-subdirectory check falsely flagged missing ORGDIR_* A‑Z folders only when AZ_DIR=False. By switching this test to AZ_DIR=True, the very condition that triggered the bug is no longer exercised; the pre-fix code would have passed this test too. Consider keeping (or adding) an AZ_DIR=False variant so the regression stays protected.

♻️ Suggested additional regression test for the original bug condition

def test_organize___incremental_run_with_az_dir_disabled___does_not_rebuild_from_scratch(self):
    """Regression for `#146`: second run must stay incremental when AZ_DIR=False."""
    self._create_ini_file(
        SKIPALTS=False,
        AZ_DIR=False,
        CATEGORY_DIR=True
    )

    config = self.ao_service.make_arcade_organizer_config(
        self.ini_path,
        self.base_path,
        ''
    )

    success1 = self.ao_service.run_arcade_organizer_organize_all_mras(config)
    self.assertTrue(success1)

    with patch.object(Infrastructure, 'remove_orgdir_directories') as mock_remove:
        success2 = self.ao_service.run_arcade_organizer_organize_all_mras(config)
        self.assertTrue(success2)

        self.assertEqual(mock_remove.call_count, 0, "Second run triggered a full rebuild unexpectedly")

🤖 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 `@src/test/integration/test_arcade_organizer.py` around lines 710 - 716, The
incremental regression test for
`test_organize___on_incremental_run___does_not_rebuild_from_scratch` no longer
covers the original `AZ_DIR=False` bug condition from `#146`. Restore coverage by
keeping this test on the disabled alphabetic-folder path, or add a separate
regression test around `make_arcade_organizer_config` and
`run_arcade_organizer_organize_all_mras` that runs with `AZ_DIR=False` and
asserts the second run does not call `Infrastructure.remove_orgdir_directories`.

🤖 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.

Nitpick comments:
In `@src/test/integration/test_arcade_organizer.py`:
- Around line 710-716: The incremental regression test for
`test_organize___on_incremental_run___does_not_rebuild_from_scratch` no longer
covers the original `AZ_DIR=False` bug condition from `#146`. Restore coverage by
keeping this test on the disabled alphabetic-folder path, or add a separate
regression test around `make_arcade_organizer_config` and
`run_arcade_organizer_organize_all_mras` that runs with `AZ_DIR=False` and
asserts the second run does not call `Infrastructure.remove_orgdir_directories`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: eb11d005-5d31-489d-9506-2909ff7f30b2

📥 Commits

Reviewing files that changed from the base of the PR and between 138a316 and 6fa7c75.

📒 Files selected for processing (2)

• src/test/integration/test_arcade_organizer.py
• src/update_all/arcade_organizer/arcade_organizer.py

[prenetic]

I don't think the AI nitpick applies anymore since we are shifting the signal for an ORGDIR rebuild up to the parent directory to consistently cover all configurations. The original test I wrote was no longer load-bearing under this change and was modified accordingly.