feat: full type safety pass and logspark 0.12.0 upgrade for 6.2.0

[Kydoimos97]

Upgrades logspark to 0.12.0 which removes @SingletonClass from SparkLogger, enabling clean inheritance for the first time. This unblocks proper static analysis across WrenchCL and all consumer repos.

What changed

• WrenchLogger now inherits directly from SparkLogger — the __bases__[0] runtime hack is gone
• logger is typed as WrenchLogger instead of Any — invalid method calls (e.g. logger.start_time()) are now caught by ty at check time
• SingletonClass made generic (Type[T] -> Type[T]) so the decorator no longer erases the class type through decoration
• py.typed marker added — consumers get full inline type information via PEP 561 without stubs
• Optional dep detection switched from try/except ImportError to importlib.util.find_spec across Connect/__init__.py and all pandas-optional modules (StandardizeNone, RdsServiceGateway) — more explicit, easier to test, no swallowed import side effects
• _MockPandas cleaned up — the try/except at module level removed, consuming files own their own conditional import with a TYPE_CHECKING guard
• All real ty errors fixed across ExceptionSuggestor, JsonParser, JsonSerializer, S3ServiceGateway, AwsClientHub, _boto_cache, Retryable, Deprecated, FileTyper, Image2B64, ProcessingTracker, StandardizeNone, _internal

Tests

• 145 passing, 0 failures (base venv + aws extras)
• test_optional_imports updated to patch importlib.util.find_spec instead of builtins.__import__ — the old patch target was bypassed by the new dep check
• Diff script (test_62_diffs.py) added to verify all 6.2.0 changes against an installed package — produces 3 expected failures on 6.1.0, 0 on 6.2.0

Deployment

Tag v6.2.0 pushed. PyPI publish triggered on merge. Consumer repos should upgrade logspark to ~=0.12.0 and WrenchCL to ~=6.2.0 together — the two are a paired release. WrenchCL 6.1.0 + logspark 0.12.0 is a broken combination (the __bases__[0] hack resolves to logging.Logger instead of SparkLogger).

References

• logspark 0.12.0: https://github.com/WrenchAI/LogSpark

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Wrench-Service-Bot]

PR Review

Verdict: REQUEST CHANGES

Description

Upgrades the logspark dependency from 0.11.0 to 0.12.0, which removes @SingletonClass from SparkLogger and allows WrenchLogger to inherit from it directly rather than via the __bases__[0] runtime hack. Alongside the dep bump, the PR runs a type-safety pass across 25 files: optional-dep detection switched from try/except ImportError to importlib.util.find_spec, None-default parameters annotated with Optional, cast() added for type-narrower returns, py.typed marker added for PEP 561.

Highlights

• WrenchLogger.__bases__[0] hack correctly removed — clean direct inheritance is sound
• SingletonClass is now generic (Type[T] -> Type[T]) — downstream type checking improved
• Optional[str] fixups across Deprecated, S3ServiceGateway, ExceptionSuggestor, _internal.py are all correct
• importlib.util.find_spec migration for pandas and AWS dep detection is cleaner than try/except
• _get_mime_extension return-type corrected to Optional[str] — mimetypes.guess_extension can return None
• Image2B64: binascii.Error instead of base64.binascii.Error — correct, the latter is an implementation detail
• JsonSerializer: explicit raise json.JSONDecodeError at the end of sanitize_unescaped_quotes_and_load_json_str is a correctness fix (previously fell through to implicit None return)
• Test suite correctly updated to patch importlib.util.find_spec instead of builtins.__import__

Blocking Finding

• WrenchCL/Connect/ProcessingTracker.py lines 163–164 — TTLSet = set followed by TTLSet(ttl=600) expands to set(ttl=600), which raises TypeError: set() got an unexpected keyword argument 'ttl' at class body evaluation time. ProcessingTracker will fail to import. The 145-test green pass is consistent with no test directly exercising this class. See inline comment for two fix options.

Style Notes

• WrenchCL/Connect/__init__.py — the re-raised ImportError drops the from e chain present in the old code. The new raise loses the original error context, making root-cause debugging harder when an import fails for an unexpected reason. Consider: raise ImportError(...) from None at minimum, or preserve the chain by catching inside the guarded block.
• .gitignore — missing newline at end of file (cosmetic).

Overall Assessment

The logspark upgrade and type-safety pass are well-scoped and the majority of changes are sound. One import-time crash in ProcessingTracker blocks merge: set(ttl=600) is not valid Python and will fail before any tests can exercise the class. Fix with a one-class stub or by dropping the ttl kwarg at the usage site, then this is ready to ship.

Sources

• Diff: 25 files, +165/-154
• Prior reviews: none
• Related PRs/issues: logspark 0.12.0 linked in PR body; no cross-repo consumer PRs checked (WrenchCL is a library — consumer upgrade is the caller's responsibility per PR notes)

— Claude

Blocking finding fixed: TTLSet stub now accepts ttl kwarg correctly; error chain suppression added.

[Wrench-Service-Bot]

PR Review

Verdict: APPROVED

Description

Re-review following REQUEST CHANGES on commit 2b52922. This commit resolves the one blocking issue flagged in that pass and adds a batch of cleanup deletions (legacy _Internal/Logging/Api/ stub files and __pycache__ binaries that should not have been tracked).

Highlights

• TTLSet crash resolved — TTLSet = set replaced with a proper class TTLSet(set) stub that accepts ttl as a keyword argument. ProcessingTracker imports and instantiates cleanly. Thread resolved.
• Relative imports in ProcessingTracker.py corrected — from Connect import AwsClientHub and from Decorators import SingletonClass fixed to from ..Connect / from ..Decorators. These were absolute-style references that would have failed in a standard package install.
• raise ImportError(...) from None in Connect/__init__.py — the style note from the prior review was acted on; from e chain is now explicitly suppressed with from None, which is the correct choice for a user-facing error message.
• JsonSerializer.sanitize_unescaped_quotes_and_load_json_str now raises json.JSONDecodeError on fallthrough — this was a latent silent None-return bug, now correct.
• recur_parse_json branch restructure is logically equivalent and the added terminal return d is unreachable but satisfies the type checker cleanly.

Style Notes

• AwsClientHub.py, RdsServiceGateway.py, ExceptionSuggestor.py — several assert x is not None guards were added as type-narrowing hints. These are stripped under python -O. For runtime invariants, if x is None: raise RuntimeError(...) is safer. Given the library context and that these are purely narrowing asserts (the surrounding logic ensures the invariant), this is advisory only — does not block merge.
• .gitignore — still missing a trailing newline at end of file (carried from prior commit, cosmetic).
• __pycache__/*.pyc deletions in the diff confirm previously tracked binaries are being cleaned up — correct direction, but __pycache__/ should be verified as gitignored going forward.

Overall Assessment

The single blocker from review #1 is cleanly resolved. All other changes reviewed in the prior pass remain sound. The type-safety sweep (logspark 0.12.0 upgrade, WrenchLogger direct inheritance, SingletonClass generics, importlib.util.find_spec migration, Optional annotations, py.typed marker) is correct and ready to ship as v6.2.0.

Sources

• Diff: 76 files, +169/-228 (bulk of additions/deletions are stub-file cleanup and .pyc removals)
• Prior reviews: 1 (REQUEST CHANGES, commit 2b52922, review #4511249552 — dismissed by author)
• Related PRs/issues: logspark 0.12.0 linked in PR body; no cross-repo consumer PRs required (WrenchCL is a library — consumer upgrade is caller's responsibility per PR notes)

— Claude