Skip to content

feat(scan): add PostHog telemetry for scan entry points - #2600

Draft
kevinmessiaen wants to merge 24 commits into
mainfrom
cursor/scan-posthog-telemetry-91d2
Draft

feat(scan): add PostHog telemetry for scan entry points#2600
kevinmessiaen wants to merge 24 commits into
mainfrom
cursor/scan-posthog-telemetry-91d2

Conversation

@kevinmessiaen

Copy link
Copy Markdown
Member

Summary

Adds PostHog telemetry to the three scan entry points in giskard-scan, following the same started/finished event pattern used by suite, scenario, and test-case runners in giskard-checks.

Events

Entry point Started Finished
quality_scan scan_quality_run_started scan_quality_run_finished
vulnerability_scan scan_vulnerability_run_started scan_vulnerability_run_finished
third_party_scan scan_third_party_run_started scan_third_party_run_finished

Each event carries aggregate, non-PII shape properties (generator counts/types, language count, target mode, parallel settings, pass/fail/error counts, duration).

Implementation

  • New _telemetry_props.py mirrors giskard-checks/_telemetry_props.py for scan-specific dimensions.
  • Each scan wraps its run in telemetry_run_context() with giskard_component / giskard_operation tags.
  • Suite-level telemetry from suite.run() continues to fire inside the scan context (nested scopes are supported).

Verification

  • make check — passed
  • make test-unit PACKAGE=giskard-scan — 162 passed, 25 skipped
Open in Web Open in Cursor 

kevinmessiaen and others added 23 commits July 2, 2026 15:00
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…udgeGenerator.generate

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Import BaseGenerator from giskard.agents (package root) instead of the deep
giskard.agents.generators.base path, satisfying the scan import-surface lint.
Document LLM-judge auto-wiring and keyless-detector skip behavior in the README.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
- Lazy-import GarakScanAdapter in third_party_scan (garak is optional)
- Defer make_judge_detector import until detector resolution
- Handle None target outputs without stringifying to "None"
- Catch unexpected detector load failures without aborting scan
- Use direct garak config assignment and Iterable type for skipped
- Fix Trace.for_target PEP 695 type parameter scoping

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
…#2583)

* fix(scan): keep garak optional for CI checks and add garak-test group

Garak stays out of dev/test dependency groups so make check (pip-audit,
licensecheck on giskard[full]) is not polluted by garak's transitive tree.

- Add garak-test dependency group, Makefile targets, and integration CI job
- Gate garak-required test modules with pytest.importorskip / skipif
- Run giskard-scan unit tests from tests/ only (exclude optional src modules)
- Configure pyright to tolerate optional garak imports in integration code
- Fix Trace.for_target TypeVar shadowing for basedpyright

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* fix(scan): typecheck garak paths with reportMissingImports only disabled

Include garak integration tests in basedpyright instead of excluding them.
Apply reportMissingImports=none only under garak src/test paths (listed
before broader giskard-scan roots so the override takes effect).

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* fix(scan): address Gemini review on garak test collection

- Simplify Makefile test-unit back to libs/$(PACKAGE) paths
- Restore src in giskard-scan testpaths for doctest coverage
- Add package-level collect_ignore for optional garak integration
  (--ignore in addopts alone fails when pytest is invoked from repo root)

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* refactor(scan): scope test-unit to tests/ dir, drop collect_ignore workaround

Use libs/$(PACKAGE)/tests for all packages except giskard-checks, which
needs the full package path for src doctests. Removes the package-level
conftest collect_ignore and src in giskard-scan testpaths — neither was
needed once collection is scoped to tests/.

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* refactor: unify test-unit to package-dir tests+src for all libs

Run pytest from each package directory so per-package pyproject addopts
(ignore garak, doctest-modules) apply correctly. Drops the giskard-checks
Makefile exception while keeping all 720 checks tests including src doctests.

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
* fix(scan): copy garak probe tags onto scenario results

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* fix(scan): use getattr for probe tags and fix e2e typecheck

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* refactor(scan): use probe.tags directly and hoist garak e2e imports

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Garak keys detector results by the short plugin name from the probe
configuration (e.g. always.Fail, judge.Refusal), not detector.name.
Populate check_name and detector details with that label so scan output
no longer shows generic check_1, check_2 names.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
* chore(scan): add lidar as git-pinned dev dependency

Wire the private Giskard-AI/lidar scanner (tag v0.2.7) as a dev-only
dependency-group (lidar-test), not a public optional-extra. Add Makefile
install-lidar-test/test-lidar targets and ignore the lidar integration
dir in giskard-scan doctests. lidar caps numpy<2.5, downgrading the
workspace lock to 2.4.6 (giskard-scan suite still green: 157 passed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): add lidar package skeleton with private-install guard

Create the giskard.scan.integrations.lidar package with lidar_available()
/ _require_lidar() mirroring the garak guard. _require_lidar raises an
ImportError explaining lidar is private and giving the git+https install
command. LidarScanAdapter is a stub, filled in by later tasks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): rebuild scan Trace from lidar attempt messages

Add async _trace_from_messages: pair each user turn with the assistant
reply that follows into an Interaction, skip system/tool messages, and
leave a trailing unmatched user turn with outputs=None. Trace.from_
interactions is async in this codebase, so the helper and its tests are
async too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): map lidar ScanResult into a scan SuiteResult

Add _to_suite_result (async), _attempt_to_check, _execution_to_check and
_SEVERITY_SCORE. One lidar Attempt becomes one ScenarioResult; the attack-
succeeded polarity is flipped (attempt.successful -> CheckResult.failure).
Severity maps to a Metric; probe/attempt evidence is carried into details
(check_name, probe_id, severity, metadata). Errored/skipped probes (result
is None) surface as a single error/skip scenario instead of crashing. An
unknown severity label degrades to no metric rather than KeyError.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): bridge scan Target into lidar and wire third_party_scan

Add ScanTargetGenerator, a giskard.agents BaseGenerator whose _call_model
drives the scan Target through the Interact/Trace seam and round-trips a
thread_id in the response metadata so lidar's multiturn probes (GOAT,
Crescendo) can thread stateful sessions. Each thread accumulates its own
frozen Trace. LidarScanAdapter.run() builds the bridge, awaits lidar's
run_scan -> wait_for_completion -> scan_result, and maps it to a
SuiteResult; a total failure degrades to an empty suite. Register the
"lidar" tool in the scan entry point. All lidar imports stay lazy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(scan): make lidar bridge survive target serialization

lidar's scanner calls target.model_dump(mode="json") to record run
metadata; a scan Target is often a plain function, so exposing it as a
pydantic field broke the whole scan with "Unable to serialize unknown
type". Store the scan Target as a PrivateAttr (excluded from model_dump,
preserved across model_copy) so scans run. Document that
discover_target_info is off, so probes needing TargetInfo report SKIP.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(scan): add lidar end-to-end functional test

Drive a real lidar scan through the bridge. The offline test runs the full
chain (bridge -> scanner -> result mapping) without a key: the probe SKIPs
under discover_target_info=False, but the scan still returns a valid
SuiteResult with traces and check_name/probe_id details. A second,
key-gated test enables discovery and asserts a real probe verdict; it
skips where no OpenAI key is available.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(scan): document lidar options in third_party_scan

The entry-point docstring only described garak's kwargs. Add lidar's
probes/tags options and note that probes generally SKIP without target
discovery (which the integration does not enable).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): add required description to third_party_scan

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* feat(scan): build lidar TargetInfo from caller description

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* test(scan): assert lidar produces a real verdict from description

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* docs(scan): add required description to third_party_scan README example

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* fix(scan): satisfy basedpyright for lidar integration

Add lidar src+tests execution environments to pyrightconfig.json (mirroring
garak) so the private lidar imports don't trip reportMissingImports in CI, and
annotate the generic-type sites the way the garak integration does. Fixes the
misplaced pyright-ignore on the unknown-tool test after description became
required.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* fix(scan): coerce query_norm to float for numpy<2.5 type check

Adding lidar caps numpy<2.5, which downgrades the workspace lock to numpy
2.4.6. Under 2.4.6's stubs np.linalg.norm returns floating[Any], not float, so
_closest_indices' float|None parameter no longer type-checks. Wrap the norm in
float() — correct for a scalar and valid under both numpy 2.4.6 and 2.5.0.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* chore(scan): regenerate THIRD_PARTY_NOTICES for numpy 2.4.6

lidar's numpy<2.5 cap downgraded the lock to numpy 2.4.6; refresh the notices
so check-notices matches the resolved dependency.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X4oWu39fVFBqmxzgYzrRHr

* refactor(scan): propagate lidar scan failures and validate kwargs

Address Gemini review on the lidar integration:

- lidar: let run_scan failures propagate instead of swallowing them into
  an empty (falsely-passing) SuiteResult, matching the garak adapter.
- both adapters: reject unexpected kwargs so caller typos (e.g. probe vs
  probes) raise TypeError instead of being silently dropped. Extracted the
  shared check into integrations/_shared.reject_unexpected_kwargs; each
  adapter still owns which kwargs are valid via its own pop() calls.
- lidar tests: add a target fixture and _patch_run_scan helper, removing
  duplicated setup across the run() tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BzUi8GmbQZToZ4BgCdasru

* refactor(scan): type lidar adapter run signature and use ChatMessage

Replace the TYPE_CHECKING-only lidar Message import with the native
giskard.llm.types.ChatMessage in _trace_from_messages, and add explicit
type parameters and annotations to LidarScanAdapter.run.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): capture lidar interactions keyed by target call_id

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): rebuild lidar trace from target_calls, fall back to messages

* test(scan): cover structured trace round-trip through model_copy

Add regression test that verifies the dict-sharing invariant in the lidar
integration: the bridge's model_copy must preserve __pydantic_private__
by reference so the adapter can rebuild traces from the shared _by_call_id
dict. The test composes the two REAL halves (capture + join) and asserts
identity of interactions, not reconstruction.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(scan): honor target_mode=singleturn for lidar by skipping multiturn probes

Lidar exposes no structural multiturn marker, and its tags_filter is
inclusion-only, so resolve the probe class set and drop any carrying the
gsk:probe-type='multi-turn' tag (crescendo, goat). Mirrors garak's
IterativeProbe skip. A guard test pins the tag so a lidar rename fails loud.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(scan): satisfy basedpyright in lidar trace tests

response is lidar's Response subclass at runtime (carries _target_call /
message) but is statically typed as the base CompletionResponse; suppress the
attribute-access errors. Annotate _EmptyBridge._by_call_id as dict[str, object].

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Update libs/giskard-scan/src/giskard/scan/integrations/_entry_point.py

Co-authored-by: Henrique Chaves <44180294+henchaves@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Henrique Chaves <44180294+henchaves@users.noreply.github.com>
…ed executor

Adds a regression test that reproduces the deadlock a shared thread pool caused:
each garak probe worker blocks on the scan loop via run_coroutine_threadsafe, and
for a structured target that loop-bound coroutine issues an LLM call whose own
work needs a pool thread. With >12 probes on asyncio.to_thread's default pool
(min(32, cpu+4)), all workers block on the loop while the loop waits for a free
thread. The test drives 40 loop-blocking probes and asserts the run completes
(fails with a 30s timeout against the pre-fix asyncio.to_thread implementation).

Also refactors the dedicated ThreadPoolExecutor from try/finally + shutdown into
a `with` block; the TaskGroup joins every probe before the executor's __exit__
shuts it down, so the context manager is equivalent and cleaner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QkTvVSF1vNs7XYttK6sTwk
Co-authored-by: David Berenstein <david.m.berenstein@gmail.com>
Mirror the suite/scenario telemetry pattern for quality_scan,
vulnerability_scan, and third_party_scan with started/finished
events and aggregate non-PII shape properties.

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces telemetry tracking for quality, vulnerability, and third-party scans within giskard-scan, capturing non-identifying execution properties (such as scan kind, generator types, and duration) at the start and end of each run. It also disables telemetry during testing and adds comprehensive unit tests. One issue was identified in generator_type_counts where generator elements can be either classes or instances; calling type(generator).__name__ on a class returns "type", which would corrupt telemetry data. A check should be added to handle both classes and instances correctly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +11 to +12
def generator_type_counts(generators: list[ScenarioGenerator]) -> dict[str, int]:
return dict(Counter(type(generator).__name__ for generator in generators))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The generators list returned by the registries can contain both generator classes (e.g., HallucinationScenarioGenerator) and generator instances (e.g., GCGInjectionScenarioGenerator()). Calling type(generator).__name__ on a class returns "type" instead of the actual class name, which will corrupt the telemetry data.

We should check if the generator is a class (using isinstance(generator, type)) and use generator.__name__ in that case, falling back to type(generator).__name__ for instances.

def generator_type_counts(generators: list[ScenarioGenerator | type[ScenarioGenerator]]) -> dict[str, int]:
    return dict(
        Counter(
            generator.__name__ if isinstance(generator, type) else type(generator).__name__
            for generator in generators
        )
    )

Use shared scan_run_started/finished events with a scan_type tag
across quality, vulnerability, and third-party scans. Include
languages, group_by, seed, probe ids, and document counts instead
of only aggregate booleans.

Co-authored-by: Kevin Messiaen <kevinmessiaen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants