Fix Regressions cause by Copilot Auto Fix by tay0thman · Pull Request #3259 · pyrevitlabs/pyRevit

tay0thman · 2026-04-03T21:13:34Z

Copilot Hallucinated when Auto fixed previous PR,

Five changes from the merged version:

_ensure_path_registered() line 59 — Replaced user_config.thirdparty_ext_root_dirs (doesn't exist) with user_config.core.get_option(CONSTS.ConfigsUserExtensionsKey, ...) which reads the raw config without filtering by op.exists()
init() line 174 — _get_default_ext_dir() instead of raw [0]
_update_add_custom_section_for_new() line 430 — same
install_custom_extension() catalog path, line 490 — same
install_custom_extension() Git URL path, line 542 — same

tay0thman · 2026-04-03T21:14:11Z

@devloai Please review and report back.

devloai

Review Summary

The five regressions described in the PR description are correctly identified and the fixes look sound overall:

✅ _ensure_path_registered() — correctly replaces the hallucinated user_config.thirdparty_ext_root_dirs attribute with a direct user_config.core.get_option(CONSTS.ConfigsUserExtensionsKey, ...) call. The exception scope is appropriately broadened to except Exception.
✅ __init__(), _update_add_custom_section_for_new(), and both paths in install_custom_extension() — correctly replace the index-zero-access pattern get_thirdparty_ext_root_dirs(include_default=True)[0] with _get_default_ext_dir(), which is safe against an empty list.
✅ _get_default_ext_dir() itself is a clean, defensive helper with a proper fallback to THIRDPARTY_EXTENSIONS_DEFAULT_DIR.

One issue found — see inline comment. The read-side fix in _ensure_path_registered correctly preserves offline paths, but the write-back on line 71 calls set_thirdparty_ext_root_dirs(), which raises PyRevitException for any path that doesn't exist on disk — effectively undoing the benefit of the raw-read for users with offline network shares.

Suggestions

Fix write-back in _ensure_path_registered to bypass existence check for offline paths Apply

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py

devloai

PR Summary:

This PR fixes regressions introduced by a Copilot Auto Fix in a prior PR. The five corrections replace non-existent user_config.thirdparty_ext_root_dirs references with core.get_option(CONSTS.ConfigsUserExtensionsKey, ...), and replace direct [0] indexing of get_thirdparty_ext_root_dirs() results with a safe helper _get_default_ext_dir().

Review Summary:

The mechanical fixes are correct — the hallucinated attribute is gone and the index-without-bounds-check is gone. However, there is a logical contradiction in _ensure_path_registered: the raw config is read via core.get_option specifically to preserve offline network shares, but the write-back goes through set_thirdparty_ext_root_dirs(), which raises PyRevitException for any path that doesn't currently exist on disk (userconfig.py:750-752). This completely undoes the offline-preservation intent — if an offline share is in the list, the new path registration fails entirely. The fix is to write back symmetrically via user_config.core.set_option(CONSTS.ConfigsUserExtensionsKey, raw_dirs). There's also a silent except Exception: with no logging on the fallback path, which hides read failures from diagnostics.

Suggestions

Update _ensure_path_registered write-back to use user_config.core.set_option(CONSTS.ConfigsUserExtensionsKey, raw_dirs) directly, matching the core.get_option read and preserving offline paths end-to-end. Apply

⚡ Apply all quick fixes (2 quick fixes)

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py

Use core.set_option() instead of set_thirdparty_ext_root_dirs() when registering extension paths. This avoids the existence check in set_thirdparty_ext_root_dirs() which would fail for temporarily-offline network shares, defeating the purpose of reading the raw list. Fixes the issue identified in review: set_thirdparty_ext_root_dirs() raises PyRevitException for any path that doesn't currently exist on disk.

…egistered - Move CONSTS import outside try block so it's available for both the read and write paths regardless of whether an exception occurs. - Replace set_thirdparty_ext_root_dirs() with core.set_option() to avoid the existence-check rejection of temporarily-offline network shares when writing back the updated directory list. - Capture the read exception and log it at debug level before falling back to the helper, replacing the silent bare 'except Exception' swallow.

devloai · 2026-04-03T21:19:38Z

Fix Summary

Applied two fixes to extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py in commit 418b18d:

Fix 1 — Offline-path preservation negated by write path (#3034420588)

The CONSTS import was moved outside the try block (so it's always available for both the read and write paths), and the write now uses user_config.core.set_option(CONSTS.ConfigsUserExtensionsKey, ...) directly instead of user_config.set_thirdparty_ext_root_dirs(). This avoids the existence check in the latter that would raise PyRevitException for any offline/non-existent path, defeating the whole purpose of reading the raw configured list. The write is also wrapped in its own try/except with logger.error (as the remote branch had already added).

Fix 2 — Silent exception swallowing hides config read failures (#3034420590)

The bare except Exception: block was changed to except Exception as read_err: and a logger.debug(...) call was added before falling back to the helper, making config read failures diagnosable without silently swallowing them.

tay0thman · 2026-04-03T21:23:54Z

@devloai Assume that I did the fixes, review the whole script and report back

devloai

Full Script Review — `Extensions.smartbutton/script.py`

The five Copilot-regression fixes are correct and well-targeted:

Fix	Assessment
`_ensure_path_registered`: raw read via `core.get_option`	✅ Correct — avoids the `op.exists()` filter
`_ensure_path_registered`: raw write via `core.set_option`	✅ Correct — bypasses `set_thirdparty_ext_root_dirs`'s existence check that would defeat offline-path preservation
Exception changed from bare `except AttributeError` → `except Exception as read_err` + `logger.debug`	✅ Correct improvement to observability
Write wrapped in its own `try/except` with `logger.error`	✅ Correct — installation should not silently fail
`[0]` → `_get_default_ext_dir()` in 4 call sites	✅ Correct — eliminates the IndexError on empty list

Two minor issues flagged:

from pyrevit.userconfig import CONSTS inside function body (line 60) — CONSTS is a module-level symbol in userconfig.py; move it to the top-level import at line 15.
Missing os.path.expandvars() on raw read (line 63) — get_thirdparty_ext_root_dirs applies expandvars before comparing; the raw read does not, so env-variable-style paths like %APPDATA%\extensions won't match on comparison and will be double-registered.

Overall the core logic is sound and the stated regressions are resolved. 🎉

Suggestions

Move CONSTS import to module level (line 15) Apply
Apply os.path.expandvars() when reading raw dirs (line 63) Apply

⚡ Apply all quick fixes (2 quick fixes)

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py

devloai · 2026-04-03T21:35:26Z

Fix Summary

Applied two fixes to extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py:

Fix 1 — Move `CONSTS` to module-level import (comment #3034445665)

Added CONSTS to the existing module-level import: from pyrevit.userconfig import user_config, CONSTS
Removed the redundant from pyrevit.userconfig import CONSTS that was inside the _ensure_path_registered function body (previously executed on every call)

Fix 2 — Expand environment variables in `raw_dirs` before normalization (comment #3034445672)

Changed raw_dirs = list(user_config.core.get_option(...)) to use a list comprehension that calls os.path.expandvars(d) on each entry, ensuring paths like %APPDATA%\pyRevit-extra are fully resolved before normalization. This prevents double-registration when the stored paths contain environment variable references.

tay0thman · 2026-04-03T22:01:37Z

Edits Tested locally and works fine. @jmcouffin

github-actions · 2026-04-04T07:38:24Z

📦 New work-in-progress (wip) builds are available for 6.3.0.26094+0727-wip

github-actions · 2026-04-05T06:57:09Z

📦 New work-in-progress (wip) builds are available for 6.3.0.26095+0645-wip

github-actions · 2026-04-05T08:41:58Z

📦 New public release are available for 6.3.0.26095+0830

Fix Regressions cause by Copilot Auto Fix

9759223

devloai bot reviewed Apr 3, 2026

View reviewed changes

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py Show resolved Hide resolved

devloai bot reviewed Apr 3, 2026

View reviewed changes

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py Outdated Show resolved Hide resolved

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py Outdated Show resolved Hide resolved

devloai bot added 2 commits April 3, 2026 21:18

devloai bot reviewed Apr 3, 2026

View reviewed changes

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py Outdated Show resolved Hide resolved

extensions/pyRevitCore.extension/pyRevit.tab/pyRevit.panel/Extensions.smartbutton/script.py Outdated Show resolved Hide resolved

Move CONSTS to module-level import and expand env vars in raw_dirs

589337c

jmcouffin closed this Apr 4, 2026

jmcouffin reopened this Apr 4, 2026

jmcouffin merged commit ad80bff into develop Apr 4, 2026
2 checks passed

jmcouffin deleted the tay0thman-Fix-Regressions-cause-by-Copilot-Auto-Fix branch April 4, 2026 07:22

Uh oh!

Conversation

tay0thman commented Apr 3, 2026

Uh oh!

tay0thman commented Apr 3, 2026

Uh oh!

devloai bot left a comment

Choose a reason for hiding this comment

Review Summary

Suggestions

Uh oh!

Uh oh!

devloai bot left a comment

Choose a reason for hiding this comment

PR Summary:

Review Summary:

Suggestions

Uh oh!

Uh oh!

Uh oh!

devloai bot commented Apr 3, 2026

Fix Summary

Fix 1 — Offline-path preservation negated by write path (#3034420588)

Fix 2 — Silent exception swallowing hides config read failures (#3034420590)

Uh oh!

tay0thman commented Apr 3, 2026

Uh oh!

devloai bot left a comment

Choose a reason for hiding this comment

Full Script Review — Extensions.smartbutton/script.py

Suggestions

Uh oh!

Uh oh!

Uh oh!

devloai bot commented Apr 3, 2026

Fix Summary

Fix 1 — Move CONSTS to module-level import (comment #3034445665)

Fix 2 — Expand environment variables in raw_dirs before normalization (comment #3034445672)

Uh oh!

tay0thman commented Apr 3, 2026

Uh oh!

Uh oh!

github-actions bot commented Apr 4, 2026

Uh oh!

github-actions bot commented Apr 5, 2026

Uh oh!

github-actions bot commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Full Script Review — `Extensions.smartbutton/script.py`

Fix 1 — Move `CONSTS` to module-level import (comment #3034445665)

Fix 2 — Expand environment variables in `raw_dirs` before normalization (comment #3034445672)