feat: implement dynamic analyzer discovery and risk score validation

[umran666]

Resolves TODO A.2.1-A.2.4 (Analyzer discovery) and TODO A.3.2 (Risk score assertion).

[rng1995]

The move to dynamic analyzer discovery is reasonable and removes a lot of boilerplate, and wiring requires_api_key / is_available() gating into create_graph matches the intent of the TODO it resolves. A few issues, one of which I'd consider blocking for a security tool.

1. Import failures are silently swallowed (blocking). In _discover_analyzers, a module that fails to import is caught with except Exception and logged at logger.debug(...), then skipped. Since the default log level is WARNING, a broken analyzer (syntax error, bad import, refactor mistake) silently disappears from every scan with no visible signal — the scan still completes and can report a low/zero risk while an entire detection category isn't running. That's a meaningful regression from the previous explicit-import behaviour, which failed loudly. Please log this at WARNING/ERROR (and ideally surface it in output) so a dropped analyzer is never invisible. It would also help to distinguish "module isn't an analyzer" (no ANALYZER_ID) from "an analyzer failed to import".

2. Confirm the gating attributes actually exist. The new skip logic reads getattr(mod, "is_available", None) and getattr(mod, "requires_api_key", False). The static analyzer modules don't define these (correctly — they need no key), but if the goal is to skip the LLM/semantic analyzers when no key is present, those modules must declare requires_api_key = True; otherwise they default to False and are always wired, making the gating a no-op. Please confirm the LLM-dependent analyzers actually set this.

3. Edge case: if discovery/gating ends up skipping every analyzer, wired_analyzers is empty and nothing connects to meta_analyzer. Worth a guard or at least a warning.

Non-blocking nits:

• Discovery order now depends on pkgutil.iter_modules (filesystem order) rather than the curated list, and the registry test was loosened to a set comparison. Fine if analyzer order is genuinely irrelevant (parallel fan-out), but a comment noting that ordering is no longer guaranteed would help future readers.
• Several of the new lines in graph.py carry trailing whitespace, which will likely trip ruff/lint.

Happy to re-review once import failures are visible and the requires_api_key wiring is confirmed.

[umran666]

Thanks for the review. I've pushed a commit addressing all the feedback:

1. Explicitly catching ImportError in __init__.py and logging at ERROR level so broken analyzers don't fail silently.
2. Added requires_api_key = True to the LLM/semantic analyzers.
3. Added a warning guard in graph.py if wired_analyzers ends up empty.
4. Fixed the trailing whitespace and added a comment noting that discovery order is filesystem-dependent.

[rng1995]

[Automated SkillSpector Review]

Non-blocking (formatting): this module-level assignment leaves only one blank line before def create_graph(). ruff format (Black style) enforces two blank lines before a top-level def, so ruff format --check will flag this file — please run make format. Same pattern before def _discover_analyzers() in nodes/analyzers/__init__.py.

[rng1995]

[Automated SkillSpector Review]

Re-review: blockers resolved — approving.

1. Import failures are no longer silent: _discover_analyzers catches ImportError/Exception and logs at ERROR (distinguishing import failure from "not an analyzer").
2. requires_api_key = True is now declared on the three semantic analyzers and create_graph gates them on is_llm_available(), so the skip logic is a real gate rather than a no-op.
3. An empty-wired_analyzers warning guard was added.

Non-blocking: the inserted module-level statements leave a single blank line before def create_graph() (in graph.py) and before def _discover_analyzers() (in __init__.py); ruff format wants two, so please run make format to avoid a CI format-check failure.

[rng1995]

Re-review — approving. The only change since my last approval is a style: run ruff format commit (graph.py, analyzers/__init__.py); the dynamic-analyzer-discovery logic I previously approved is unchanged.

Heads-up (non-blocking for the review, but blocks merge): GitHub currently reports this branch as conflicting with main (mergeable_state: dirty) — please rebase/resolve before merging.

[umran666]

Hey @rng1995 just checking in. All review feedback has been addressed and the latest push includes the ruff format fix. Is this good to merge, or is there anything else you'd like me to change?

[rng1995]

@umran666 - Please resolve merge conflicts, so that I can merge

[umran666]

@umran666 - Please resolve merge conflicts, so that I can merge

@rng1995 Rebased on latest main and resolved merge conflicts.Ready to merge.