feat: comprehensive AI content safety framework (text, image, audio, video) - #868
Merged
Merged
Conversation
…, facade integration Phase 1 (Core Architecture): - SafetyConfig with nested sub-configs (Text, Image, Audio, Video, Watermarking, Guardrails, Fairness, Compliance) — single ConfigureSafety() entry point - ISafetyModule<T> composable interface hierarchy (Text, Image, Audio, Video) - SafetyPipeline<T> orchestrator running modules and aggregating SafetyReport - SafetyFinding, SafetyReport, SafetyViolationException result types - SafetyCategory (40 categories), SafetyAction, SafetySeverity enums - SafetyPipelineFactory auto-constructs pipeline from config Phase 2 (Text Safety — P0): - RuleBasedToxicityDetector: regex patterns for violence, hate speech, self-harm, terrorism, CSAM, social engineering with ReDoS protection - RegexPIIDetector: email, SSN, credit card, phone, IP, API keys, AWS keys, passwords, passport, driver's license with masked excerpts - PatternJailbreakDetector: direct override, persona hijack, prompt extraction, encoding attacks (Base64, Unicode tag smuggling), multi-turn escalation Facade Integration: - AiModelBuilder.ConfigureSafety(Action<SafetyConfig>) on builder + interface - AiModelResult.SafetyPipeline property for runtime access - AiModelResult.EvaluateTextSafety() convenience method - AiModelResult.GetSafetyConfig() for config inspection Closes #287 (partial — Phase 1 + Phase 2 text modules) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a comprehensive Safety subsystem (configs, enums, pipeline, builder/factory, report/finding types, violation exception), many modality-specific interfaces/base classes, dozens of concrete detectors and guardrails across text/image/audio/video/multimodal, benchmarking and compliance tooling, AiModelBuilder/IAiModelBuilder.ConfigureSafety API, and AiModelResult safety hooks. All changes are additive. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as rgba(30,144,255,0.5)
participant Builder as rgba(34,139,34,0.5)
participant Factory as rgba(255,140,0,0.5)
participant Pipeline as rgba(138,43,226,0.5)
participant Modules as rgba(220,20,60,0.5)
participant Result as rgba(70,130,180,0.5)
Client->>Builder: ConfigureSafety(configureAction)
Client->>Builder: Build()
Builder->>Factory: Create(safetyConfig)
Factory->>Pipeline: new SafetyPipeline(config)
Factory->>Modules: AddModule(...) (guardrails/text/image/audio/video/...)
Factory-->>Builder: SafetyPipeline<T>
Builder->>Result: AttachSafetyPipeline(result)
Client->>Result: EvaluateTextSafety(text)
Result->>Pipeline: EvaluateText(text)
Pipeline->>Modules: for each ready ITextSafetyModule -> EvaluateText
Modules-->>Pipeline: SafetyFinding[]
Pipeline->>Pipeline: Aggregate -> SafetyReport
Pipeline-->>Result: SafetyReport
Result-->>Client: SafetyReport / EnforcePolicy
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive, modular safety framework replacing the monolithic ISafetyFilter with a composable pipeline architecture. The implementation provides a single facade method ConfigureSafety() on AiModelBuilder and delivers three operational text safety modules (toxicity, PII, jailbreak detection) with ReDoS-protected regex patterns, supported by 72 research paper citations spanning 2024-2026.
Changes:
- Replaced monolithic safety filter with composable
ISafetyModule<T>architecture supporting text, image, audio, and video modalities - Added
ConfigureSafety()facade method with nested configuration for all safety domains - Implemented three text safety modules with pattern-based detection and 100ms regex timeouts
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Safety/Text/TextSafetyModuleBase.cs |
Base class providing common vector-to-text conversion for text safety modules |
src/Safety/Text/RuleBasedToxicityDetector.cs |
Pattern-based toxicity detector with curated regex rules for harmful content |
src/Safety/Text/RegexPIIDetector.cs |
PII detector using regex patterns for emails, SSNs, credit cards, API keys |
src/Safety/Text/PatternJailbreakDetector.cs |
Jailbreak detector catching prompt injection, DAN attacks, and encoding-based exploits |
src/Safety/SafetyViolationException.cs |
Exception thrown when content fails safety checks with throw-on-violation enabled |
src/Safety/SafetyReport.cs |
Aggregated safety report with overall verdict, severity, and action recommendations |
src/Safety/SafetyPipelineFactory.cs |
Factory constructing SafetyPipeline<T> from SafetyConfig |
src/Safety/SafetyPipeline.cs |
Runtime orchestrator executing safety modules and aggregating findings |
src/Safety/SafetyFinding.cs |
Individual safety finding with category, severity, confidence, and span information |
src/Safety/SafetyConfig.cs |
Master configuration with nested sub-configs for all safety domains |
src/Models/Results/AiModelResult.cs |
Added SafetyPipeline property and EvaluateTextSafety() convenience method |
src/Interfaces/IVideoSafetyModule.cs |
Interface for video content safety modules |
src/Interfaces/ITextSafetyModule.cs |
Interface for text content safety modules |
src/Interfaces/ISafetyModule.cs |
Base interface for all composable safety modules |
src/Interfaces/IImageSafetyModule.cs |
Interface for image content safety modules |
src/Interfaces/IAudioSafetyModule.cs |
Interface for audio content safety modules |
src/Interfaces/IAiModelBuilder.cs |
Added ConfigureSafety() method signature |
src/Enums/SafetySeverity.cs |
Severity levels from Info to Critical |
src/Enums/SafetyCategory.cs |
40 safety categories covering all content types |
src/Enums/SafetyAction.cs |
Actions from Allow to Quarantine |
src/AiModelBuilder.cs |
Implementation of ConfigureSafety() and pipeline construction |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/AiModelBuilder.cs`:
- Around line 2700-2704: ConfigureSafety() currently only attaches the safety
pipeline inside the supervised build path; centralize the attachment so every
result creation path uses it. Move the logic that sets
finalResult.SafetyPipeline =
AiDotNet.Safety.SafetyPipelineFactory<T>.Create(_safetyPipelineConfig) into a
single helper (e.g., ApplySafetyPipeline(result) or inside ConfigureSafety())
and call that helper from all result constructors and deserialization code paths
(supervised, RL, streaming, meta-learning, program-synthesis inference-only, and
any Deserialize* method), guarding on _safetyPipelineConfig being non-null so
behavior is identical.
In `@src/Models/Results/AiModelResult.cs`:
- Around line 651-670: The public SafetyPipeline property
(AiDotNet.Safety.SafetyPipeline<T>? SafetyPipeline) is being serialized by
Json.NET and can leak runtime internals or fail; mark this property with the
Json.NET ignore attribute (apply [JsonIgnore] /
Newtonsoft.Json.JsonIgnoreAttribute) and add the corresponding using (or
fully-qualify the attribute) so the pipeline is omitted from serialization,
leaving the property internal-set as-is.
- Around line 1430-1462: Validate the incoming text in EvaluateTextSafety: if
text is null throw ArgumentNullException, and if text is empty or whitespace
throw ArgumentException (or choose a consistent single validation exception),
before checking SafetyPipeline; keep the existing behavior of returning
SafetyReport.Safe when SafetyPipeline is null and otherwise call
SafetyPipeline.EvaluateText(text). Update the AiModelResult.EvaluateTextSafety
method to perform this parameter validation and include the method name
(EvaluateTextSafety), the SafetyPipeline property, and the SafetyReport return
path when making the change.
In `@src/Safety/SafetyConfig.cs`:
- Line 194: EffectiveLanguages currently allocates a new string[] on each
access; change it to return a shared static readonly default array instead of
new[] {"en"} to avoid repeated allocations—add a private static readonly
string[] DefaultLanguages = new[] { "en" } (or similar name) and modify the
internal string[] EffectiveLanguages => Languages ?? DefaultLanguages so it
mirrors other Effective* properties and avoids per-access allocation; reference
the EffectiveLanguages property and the Languages field/property when making
this change.
In `@src/Safety/SafetyPipeline.cs`:
- Around line 113-146: The EvaluateText method (and likewise
EvaluateImage/EvaluateAudio/EvaluateVideo) currently only checks
_config.EffectiveEnabled; change each evaluator to also check its
modality-specific enabled flag (e.g., _config.Text.EffectiveEnabled or
_config.Text.Enabled for text, _config.Image.* for images, _config.Audio.* for
audio, _config.Video.* for video) and return
SafetyReport.Safe(Array.Empty<string>()) immediately if that modality is
disabled; keep the rest of the logic (Stopwatch timing, collecting modulesRun
and aggregating findings from ITextSafetyModule<T> / IImageSafetyModule /
IAudioSafetyModule / IVideoSafetyModule) unchanged so the gate prevents running
modules when the modality is turned off.
In `@src/Safety/SafetyPipelineFactory.cs`:
- Around line 38-42: Remove the "// Future: RegisterImageModules,
RegisterAudioModules, RegisterVideoModules" placeholder and instead add an
explicit guard in the SafetyPipelineFactory method that builds the pipeline (the
code that calls RegisterTextModules(pipeline, effectiveConfig)): inspect
effectiveConfig for enabled modalities other than text and immediately throw a
clear NotSupportedException/InvalidOperationException listing which unsupported
modalities are enabled (e.g., "Image/Audio/Video safety not implemented: [Image,
Audio]"). This makes the failure fast and obvious; do not leave silent
placeholders — either implement
RegisterImageModules/RegisterAudioModules/RegisterVideoModules or throw when
those modalities are requested.
- Around line 21-29: The class SafetyPipelineFactory<T> is currently public but
should be internal to hide plumbing from consumers; change its declaration to an
internal static class (i.e., make SafetyPipelineFactory<T> internal) so only
internal code can use it, and update any internal references/usages (e.g., calls
from AiModelBuilder or related factory methods) to remain accessible within the
assembly; ensure no external callers depend on the public type before changing
visibility.
In `@src/Safety/SafetyReport.cs`:
- Around line 77-157: The factory methods Safe and FromFindings must defensively
validate and copy inputs: ensure modulesExecuted and findings are checked for
null (treat null as empty), and snapshot them (e.g., copy to arrays/lists)
before storing in the returned SafetyReport to avoid external mutation; in
FromFindings also validate each finding is non-null and clamp finding.Confidence
to the [0.0,1.0] range when computing severity-weighted score so OverallScore
stays within 0–1, and use the clamped value in the
weightedScore/minConfidenceWeightedScore math; finally set Findings and
DetectedCategories on the new SafetyReport to the copies (not the original
collections).
In `@src/Safety/SafetyViolationException.cs`:
- Around line 37-42: The constructor for SafetyViolationException should guard
against a null SafetyReport before calling BuildMessage; change the base call in
the SafetyViolationException(SafetyReport report, bool isInputViolation)
constructor to use a null-coalescing throw (e.g., BuildMessage(report ?? throw
new ArgumentNullException(nameof(report)), isInputViolation)) so an
ArgumentNullException is thrown immediately and BuildMessage is never invoked
with a null report; keep setting Report = report and IsInputViolation =
isInputViolation as before.
In `@src/Safety/Text/PatternJailbreakDetector.cs`:
- Around line 155-182: The sensitivity boundary checks in
PatternJailbreakDetector.cs currently use <= which excludes exactly 0.5 and 0.3
from medium/aggressive pattern groups; change the two checks in the method that
constructs patterns (the if checks that read "if (sensitivity <= 0.5)" and "if
(sensitivity <= 0.3)") to use "<" instead so sensitivity values 0.5 and 0.3 are
included in the medium and aggressive pattern sets respectively (update the
comparisons in the method that builds the patterns in the
PatternJailbreakDetector class).
- Around line 243-266: The current ContainsUnicodeTagCharacters method
incorrectly treats characters in '\uFFF0'..'\uFFFF' as tag characters, causing
false positives; remove the broad BMP check and rely solely on detecting
surrogate pairs and converting them to code points (using
char.IsHighSurrogate/char.IsLowSurrogate and char.ConvertToUtf32) to test for
the actual Unicode Tag block range U+E0001..U+E007F (i.e., update the loop that
examines surrogate pairs in ContainsUnicodeTagCharacters to be the only check
and ensure it properly iterates over the string without double-counting
surrogates).
In `@src/Safety/Text/RuleBasedToxicityDetector.cs`:
- Around line 42-66: The constructor currently accepts and stores
confidenceThreshold (private field _confidenceThreshold) but never uses it when
emitting findings (they always have Confidence=1.0), so remove the misleading
unused parameter and related validation: change the RuleBasedToxicityDetector
constructor to a parameterless ctor, delete the _confidenceThreshold field and
its validation/assignment, and update the XML doc/comments to reflect no
threshold; ensure no other code references _confidenceThreshold (remove or
refactor such references if present) and keep BuildDefaultPatterns and existing
behavior unchanged.
In `@src/Safety/Text/TextSafetyModuleBase.cs`:
- Around line 40-52: Add a null-check at the start of Evaluate(Vector<T>) to
fail fast: if content is null throw new ArgumentNullException(nameof(content)).
This ensures Evaluate(Vector<T>) (which calls
MathHelper.GetNumericOperations<T>() and eventually EvaluateText) doesn't
dereference a null Vector and provides a clear exception to callers; place the
guard before computing numOps or accessing content.Length.
…compliance, and benchmarking safety modules Implements the remaining safety pipeline phases for issue #287: - Image safety: CLIP-based NSFW/violence classifier - Audio safety: spectral deepfake detector, transcription toxicity detector - Video safety: frame-sampling moderator, temporal consistency deepfake detector - Watermarking: text (SynthID-inspired), image (frequency-domain), audio (spread-spectrum) - Guardrails: input validation, output validation, topic restriction, custom rules - Multimodal: cross-modal consistency checker for evasion attack detection - Compliance: EU AI Act, GDPR, SOC2 compliance checkers - Benchmarking: benchmark runner with precision/recall/F1, standard test suites - Updated SafetyPipelineFactory to register all modules based on config Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 35
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Safety/Audio/AudioSafetyModuleBase.cs`:
- Around line 37-40: The constructor AudioSafetyModuleBase should validate that
defaultSampleRate is a positive integer before assigning to _defaultSampleRate;
if defaultSampleRate <= 0, throw an ArgumentOutOfRangeException (or
ArgumentException) with a clear message indicating the parameter name and that
sample rate must be > 0 so invalid values are rejected early and consistently
with other modules.
In `@src/Safety/Audio/SpectralDeepfakeDetector.cs`:
- Around line 155-164: The EstimateDeepfakeScore method currently returns a
hardcoded 0.0 which disables detection; replace the placeholder return with a
NotSupportedException to fail-fast until a real model is integrated: update
SpectralDeepfakeDetector.EstimateDeepfakeScore to throw new
NotSupportedException("Spectral deepfake scoring is not implemented") (remove
the unused discard assignment `_ = features;`) so callers immediately know the
detector is unimplemented; alternatively, if you prefer to keep a stub path,
implement a minimal heuristic using the computed features inside
ComputeSpectralFeatures and document it, but do not leave a silent 0.0 return.
- Line 38: SpectralDeepfakeDetector<T> is declared public but should be internal
to preserve the facade pattern; change the class visibility from public to
internal for SpectralDeepfakeDetector<T> so it is not directly exposed, keeping
interactions limited to AiModelBuilder and AiModelResult; ensure the type
parameter and inheritance from AudioSafetyModuleBase<T> remain unchanged and
update any unit tests or internal references to use the internal visibility (or
move callers into the same assembly) to avoid access issues.
In `@src/Safety/Audio/TranscriptionToxicityDetector.cs`:
- Around line 186-198: EstimateAcousticToxicity currently returns a hardcoded
0.0 which disables the detector; replace the stub with a deterministic heuristic
that maps AcousticFeatures (from ComputeAcousticFeatures /
ComputeShortTermEnergyVariance) to a normalized 0.0–1.0 toxicity score (or
remove the module). Specifically, implement logic inside
EstimateAcousticToxicity that reads features.pitchVariance,
features.shortTermEnergyVariance, features.spectralCentroid (or equivalent
properties on AcousticFeatures), combines them (e.g., weighted sum with clamping
and normalization) into a double in [0,1], and document the chosen weights; if
you cannot provide a heuristic now, throw a NotImplementedException (or delete
the class) instead of returning 0.0 so callers don’t silently assume no
toxicity. Ensure the function name EstimateAcousticToxicity and the
AcousticFeatures shape are used so the change integrates with existing
ComputeAcousticFeatures and ComputeShortTermEnergyVariance.
In `@src/Safety/Benchmarking/SafetyBenchmarkRunner.cs`:
- Around line 58-112: RunBenchmark currently assumes every item in the testCases
list and every SafetyBenchmarkCase.Text is non-null; add explicit validation at
the start of SafetyBenchmarkRunner.RunBenchmark to handle null entries and null
Text values (either by throwing an ArgumentException/ArgumentNullException or by
filtering/skipping invalid cases) before iterating. Ensure you reference the
RunBenchmark parameter testCases and each SafetyBenchmarkCase.Text and update
downstream code that uses report = _pipeline.EvaluateText(testCase.Text) so it
never receives a null string; also preserve current behavior of returning
SafetyBenchmarkResult.Empty when the effective set of valid cases is empty.
In `@src/Safety/Benchmarking/StandardSafetyBenchmarks.cs`:
- Around line 32-79: The properties JailbreakBenchmark, PIIBenchmark, and
ToxicityBenchmark currently use expression-bodied getters that allocate a new
array on every access; change each to a single static readonly field (e.g.,
static readonly IReadOnlyList<SafetyBenchmarkCase> JailbreakBenchmark)
initialized once with the SafetyBenchmarkCase[] literal to avoid repeated
allocations and preserve immutability; update the declarations for all three
(referencing the symbol names JailbreakBenchmark, PIIBenchmark,
ToxicityBenchmark and the element type SafetyBenchmarkCase) and ensure callers
still see an IReadOnlyList<I> view so external code behavior does not change.
- Around line 167-177: FullBenchmark currently allocates a new List and copies
from JailbreakBenchmark, PIIBenchmark, and ToxicityBenchmark on every get;
change it to compute and cache the combined IReadOnlyList<SafetyBenchmarkCase>
once (lazy-init) and return the cached instance thereafter to avoid repeated
allocations. Implement a private static field (e.g., _fullBenchmark or
Lazy<IReadOnlyList<SafetyBenchmarkCase>>) and populate it by concatenating the
three sources (JailbreakBenchmark, PIIBenchmark, ToxicityBenchmark) into a
single read-only collection on first access, ensuring thread-safety during
initialization, then have FullBenchmark return that cached collection.
In `@src/Safety/Compliance/EUAIActComplianceChecker.cs`:
- Around line 63-113: EvaluateText currently ignores its text parameter and only
checks configuration flags; either make this a configuration-only checker by
changing the implementation to a configuration checker interface or actually
analyze the incoming text. To fix without changing the interface, update
EvaluateText to run content checks when the config flags indicate capability:
call your text-analysis helpers (e.g., DetectPII(text), DetectToxicity(text),
and DetectWatermarking(text) or whichever methods exist in your codebase) and
convert their results into SafetyFinding entries instead of only using
_config.Text.EffectivePIIDetection / _config.Text.EffectiveToxicityDetection /
_config.Watermarking.EffectiveTextWatermarking; alternatively, if you prefer a
config-only role, move this logic out of the ITextSafetyModule<T> implementation
into a new IConfigurationChecker and remove the unused text parameter from
EvaluateText in the current class (or add a clear TODO/comment) so EvaluateText
no longer lies about analyzing content.
- Line 39: Change the class accessibility from public to internal for
EUAIActComplianceChecker<T> to preserve the facade pattern; locate the
declaration "public class EUAIActComplianceChecker<T> : ITextSafetyModule<T>"
and update it to use the internal modifier, and then verify any external
references or tests reference it through the intended public facade (adjust
visibility or move tests if necessary) so compilation and encapsulation remain
correct.
In `@src/Safety/Compliance/GDPRComplianceChecker.cs`:
- Around line 91-95: The Evaluate(Vector<T> content) method in
GDPRComplianceChecker currently returns an empty array and must perform real
checks per the ISafetyModule<T> contract; implement it to delegate to the
existing single-item evaluation logic (e.g., call the existing Evaluate(T item)
or a shared internal check method such as EvaluateContent/RunChecks) for each
element in the vector and aggregate the resulting SafetyFinding instances into a
single IReadOnlyList<SafetyFinding>; ensure null handling and preserve any
deduplication/priority logic used by the scalar Evaluate so behavior matches the
non-vector path.
- Around line 81-86: The empty conditional in GDPRComplianceChecker that checks
_config.Text.EffectivePIIDetection is incomplete — either implement the promised
compliance checks or reduce the docs; fix by calling the PII detector (the
existing PII detection service used elsewhere), inspect its results for detected
identifiers in the input text, then run the GDPR rule checks (e.g., Article 5
purpose/minimization, Articles 13–14 notice requirements, Article 17 erasure
triggers, Article 22 profiling/automated decision flags, and Article 35 DPIA
triggers) and populate the method's compliance findings/return value
accordingly; if you cannot implement full rule logic now, narrow the
documentation and replace the empty block with a clear no-op return or an
explicit "not implemented" compliance result so GDPRComplianceChecker no longer
silently does nothing.
In `@src/Safety/Compliance/SOC2ComplianceChecker.cs`:
- Line 40: Change the SOC2ComplianceChecker<T> class visibility from public to
internal to preserve the facade pattern; update the class declaration of
SOC2ComplianceChecker<T> (which implements ITextSafetyModule<T>) to use the
internal modifier and ensure any consumers outside the intended assembly are not
depending on the public type (or are adjusted to internal accessors via friend
assemblies if needed).
- Around line 62-127: EvaluateText currently ignores its text parameter and only
inspects configuration, which mismatches the ITextSafetyModule<T> contract;
either make EvaluateText perform per-input analysis or change its
responsibility. Fix by either (A) implementing actual text checks inside
EvaluateText (use the incoming text to run
PII/jailbreak/prompt-injection/output-validation heuristics and return findings
per input) referencing the EvaluateText method and SafetyFinding creation sites,
or (B) move this logic to a dedicated configuration validator (rename the
class/interface from ITextSafetyModule<T> usage to a config checker) and update
callers/signature so EvaluateText is removed or reworked; also add XML docs or
comments to clarify intent. Ensure references to _config, EvaluateText, and the
SafetyFinding construction points are updated accordingly.
In `@src/Safety/Guardrails/CustomRuleGuardrail.cs`:
- Around line 99-110: Replace the direct use of ex.Message in the SafetyFinding
created inside the catch block so internal exceptions are not leaked: set
Description to a generic message like "Custom rule '{rule.Name}' threw an
exception." (keep Category, Severity, Confidence, RecommendedAction,
SourceModule as-is), and send the full exception details (ex) to internal
logging/telemetry instead (e.g., ILogger or Telemetry client) from the same
catch block so diagnostic data is retained but not exposed in the SafetyFinding.
- Around line 117-121: The Evaluate(Vector<T> content) method in
CustomRuleGuardrail is currently a stub that returns an empty list and thus lets
vector inputs bypass safety; change it to fail-closed by either implementing
vector handling (iterate the Vector<T> elements, call the existing Evaluate(T
content) logic for each element or the rule evaluation logic, aggregate and
return all SafetyFinding results) or immediately throw a clear exception (e.g.,
NotSupportedException/NotImplementedException) to prevent silent acceptance;
update the method signature Evaluate(Vector<T> content), reference
CustomRuleGuardrail and SafetyFinding when adding aggregation or throwing, and
ensure the behavior matches other Evaluate overloads.
In `@src/Safety/Guardrails/OutputGuardrail.cs`:
- Around line 53-67: Validate the repetitionThreshold and minOutputLength in the
OutputGuardrail constructor: check that repetitionThreshold is > 0 and < 1 and
throw an ArgumentOutOfRangeException(nameof(repetitionThreshold)) if not, and
stop silently clamping minOutputLength—instead validate minOutputLength >= 0
(and optionally ensure minOutputLength <= maxOutputLength) and throw
ArgumentOutOfRangeException(nameof(minOutputLength)) when invalid; update
assignments to _repetitionThreshold and _minOutputLength only after these
checks.
- Around line 126-130: The Evaluate(Vector<T>) method in OutputGuardrail
currently returns an empty array which is a blocking placeholder; replace this
with a clear unsupported-operation response by throwing a NotSupportedException
with a message stating that OutputGuardrail only supports text evaluation via
EvaluateText(), or if vector evaluation is intended implement the real
vector-analysis logic in Evaluate(Vector<T>) consistent with the
ISafetyModule<T> contract (update tests and callers accordingly); locate the
method named Evaluate(Vector<T>) in class OutputGuardrail and either throw new
NotSupportedException("OutputGuardrail only supports text evaluation via
EvaluateText()") or implement the proper vector safety checks.
In `@src/Safety/Guardrails/TopicRestrictionGuardrail.cs`:
- Line 73: Rename the variable lowerText to a clearer name (e.g., preLoweredText
or lowerTextInvariant) to indicate the text has been pre-lowercased and ensure
all usages inside the TopicRestrictionGuardrail method reference this renamed
variable; keep the ToLowerInvariant() call performed once outside the loop
(where it currently is) and update any checks that use lowerText to use the new
name.
- Around line 105-108: The catch block that swallows RegexMatchTimeoutException
in TopicRestrictionGuardrail should surface observability: update the catch in
TopicRestrictionGuardrail (the RegexMatchTimeoutException handler) to log a
warning (using the class's ILogger or equivalent) including the pattern and
context, and/or increment a metric counter for regex timeout occurrences; retain
the current behavior of skipping the pattern but ensure the log/metric call is
non-throwing and includes enough context to debug slow/evil patterns.
In `@src/Safety/Image/CLIPImageSafetyClassifier.cs`:
- Around line 183-204: Both EstimateNSFWScore and EstimateViolenceScore
currently return 0.0 unconditionally, making the classifier non-functional;
replace the placeholder behavior by throwing a NotSupportedException with a
clear message that CLIP-based scoring is not yet implemented (referencing CLIP
integration pending) inside both EstimateNSFWScore(ImageStatistics stats) and
EstimateViolenceScore(ImageStatistics stats), and update any callers or the
pipeline factory to either catch this exception or remove/register the
classifier conditionally so production cannot silently rely on it.
- Line 38: Make the CLIPImageSafetyClassifier<T> type internal (not public) so
it’s hidden from consumers; locate the class declaration for
CLIPImageSafetyClassifier<T> which currently inherits ImageSafetyModuleBase<T>
and change its accessibility to internal, ensuring the SafetyPipelineFactory can
still instantiate it and that AiModelBuilder.ConfigureSafety() / AiModelResult
remain the only public safety-facing APIs; verify there are no external
references expecting it to be public and update tests or internal factories if
necessary.
In `@src/Safety/Multimodal/CrossModalConsistencyChecker.cs`:
- Line 41: Change the class visibility of CrossModalConsistencyChecker<T> from
public to internal to keep it as internal plumbing; update the type declaration
`public class CrossModalConsistencyChecker<T> : ITextSafetyModule<T>` to
`internal class CrossModalConsistencyChecker<T> : ITextSafetyModule<T>` and
ensure any registrations in SafetyPipelineFactory still compile (they can
reference the internal type within the same assembly); do not expose this type
to consumers—users should interact via AiModelBuilder.ConfigureSafety() /
AiModelResult only.
In `@src/Safety/Video/FrameSamplingVideoModerator.cs`:
- Around line 24-27: The block comment in the FrameSamplingVideoModerator class
that reads "Key-frame extraction could further optimize (not yet implemented)"
is a future‑enhancement note that must be removed; edit the XML/doc comment near
the Sampling strategy (inside FrameSamplingVideoModerator) to either delete the
parenthetical " (not yet implemented)" or remove the entire sentence about
key‑frame extraction so no future‑enhancement or TODO text remains in production
comments.
- Around line 71-78: The EvaluateVideo method currently assumes inputs are
valid; add explicit guards at the start of EvaluateVideo to validate parameters:
throw an ArgumentNullException if frames is null, and throw an
ArgumentOutOfRangeException if frameRate is less than or equal to zero (rather
than silently continuing); keep returning an empty List<SafetyFinding> only
after these checks. Reference the EvaluateVideo method, the frames parameter,
frameRate parameter, and the SafetyFinding return usage when applying the
changes.
In `@src/Safety/Video/TemporalConsistencyDetector.cs`:
- Around line 134-153: The ComputeFrameDifference method currently uses Math.Min
to silently truncate mismatched tensor data; instead validate that
frame1.Data.Span.Length equals frame2.Data.Span.Length and throw an informative
exception if they differ. In the ComputeFrameDifference function, after
obtaining span1 and span2, replace the minLength logic with an equality check
and throw an ArgumentException (or custom exception) including the differing
lengths and identifiers for frame1/frame2; keep the existing loop and averaging
logic unchanged for the case when lengths match. Ensure the exception message
references ComputeFrameDifference and the tensor lengths to aid debugging.
- Around line 156-168: The EstimateDeepfakeScore(TemporalFeatures) function
currently returns a constant 0.0; replace the stub with a deterministic
heuristic that computes a 0.0–1.0 score from TemporalFeatures fields (e.g.,
motionMagnitudeVariance, faceStabilityScore, blinkConsistency, mouthSyncScore)
by normalizing each feature to expected ranges, combining them with weighted
sums (higher motion variance and low face stability increase deepfake
likelihood; poor blink/mouth sync increase likelihood), clamping the result to
[0,1], and returning it; update the XML summary/remarks to remove the “always
0.0” placeholder and note that this is a heuristic until a real temporal model
is injected, and consider exposing an interface or parameter to inject a real
model later (keep identifiers EstimateDeepfakeScore and TemporalFeatures
unchanged).
- Around line 67-75: In EvaluateVideo, add explicit input validation: throw
ArgumentNullException if the frames parameter is null and throw
ArgumentOutOfRangeException if frameRate is <= 0, rather than silently returning
an empty findings list; keep the existing early-return for frames.Count < 2 if
desired, but ensure the null and non‑positive frameRate checks occur at the
start of the TemporalConsistencyDetector.EvaluateVideo method (referencing the
frames and frameRate parameters and the returned IReadOnlyList<SafetyFinding>).
In `@src/Safety/Video/VideoSafetyModuleBase.cs`:
- Around line 50-55: The Evaluate(Vector<T> content) method lacks validation for
a null content parameter; add an explicit guard at the start of Evaluate that
checks if content is null and throws an ArgumentNullException (mentioning the
parameter name) before calling content.ToArray(); this preserves current
behavior of creating a Tensor<T> and calling EvaluateVideo(frames,
_defaultFrameRate) while preventing a null reference exception in Tensor<T>
construction.
In `@src/Safety/Watermarking/AudioWatermarker.cs`:
- Around line 93-131: The EstimateWatermarkPresence stub currently returns 0.0
and never uses the _watermarkStrength field, so implement a real detection path:
in AudioWatermarker<T> replace the placeholder
EstimateWatermarkPresence(Vector<T> audioSamples) with a spread‑spectrum
correlation routine that (1) synthesizes the expected watermark signal (seeded
by the same key/PN sequence the watermarker uses), (2) correlates that template
with audioSamples (resample/convert to float if needed), and (3) computes and
returns a normalized detection score (e.g., correlation magnitude divided by
expected energy, clamped 0–1) which uses _watermarkStrength as the expected
watermark amplitude; update EvaluateAudio/Evaluate to rely on that score and
remove or repurpose _watermarkStrength only if it remains unused.
In `@src/Safety/Watermarking/ImageWatermarker.cs`:
- Around line 95-136: The EstimateWatermarkPresence stub currently always
returns 0.0 and _watermarkStrength is unused, which leaves ImageWatermarker
effectively non-functional; either remove this class from the PR or implement a
minimal functional detector: replace EstimateWatermarkPresence(ReadOnlySpan<T>)
with a real frequency-domain check (e.g., block-wise DCT/FFT, extract
mid-frequency coefficients, correlate against the expected watermark pattern and
return a normalized detectionScore) and ensure _watermarkStrength is consumed in
the detection logic (use it to scale the expected pattern or thresholding), and
wire the implemented detector into EvaluateImage/Evaluate so findings are
produced when detectionScore >= _detectionThreshold; update unit tests
accordingly.
In `@src/Safety/Watermarking/TextWatermarker.cs`:
- Around line 101-107: The current whitespace-only tokenization in
TextWatermarker.cs (where TokenizeSimple(text) is used) must be replaced with an
injected/model tokenizer to match the generation tokenizer: add a tokenizer
delegate or ITokenizer dependency to TextWatermarker (constructor and a field),
remove the TokenizeSimple fallback usages in the methods that call
TokenizeSimple (including the block guarded by _contextWidth and the other
occurrence around lines 165-168), and call the injected tokenizer (e.g.,
tokenizer.Tokenize(text) or delegate(text)) so detection uses the identical
tokenization as the model; ensure null-checks/assertions for the injected
tokenizer and update tests accordingly.
- Around line 158-163: The Evaluate(Vector<T> content) implementation in
TextWatermarker.cs currently returns Array.Empty and silently skips watermark
detection; change this to fail fast for unsupported input by throwing a
NotSupportedException (or InvalidOperationException) with a clear message like
"Text watermarking does not support vector inputs" from the Evaluate method so
callers get an explicit error instead of a silent false-negative; keep the
method signature and ensure any callers/tests are updated to expect/handle the
exception and that SafetyFinding is not returned for this code path.
- Around line 54-81: The TextWatermarker constructor currently uses a hardcoded
default for _secretKey; remove that default and require callers to provide a
non-null, non-empty secretKey (update the XML docs to state it is required and
must be securely generated), validate the provided byte[] in
TextWatermarker(byte[]? secretKey, ...) and throw an ArgumentNullException or
ArgumentException if null/zero-length or fails a minimum entropy/length check
(e.g., minimum length like 16 bytes), and set _secretKey = secretKey after
validation; also update the XML comments for the secretKey parameter to remove
the default-key guidance and advise secure unique keys per deployment.
- Around line 41-42: TextWatermarker<T> currently uses a hardcoded secret,
non-deterministic string.GetHashCode(), a no-op Evaluate(Vector<T>) and
simplistic whitespace tokenization; fix by removing the hardcoded key from the
constructor and accept a secret via DI/config (IOptions or constructor
injection) in the TextWatermarker<T> constructor, replace any use of
string.GetHashCode() with a deterministic hash (e.g., HMACSHA256 or SHA256) for
watermark generation/verification in the methods that compute hashes, either
implement Evaluate(Vector<T>) to mirror the string-path behavior or make it
throw NotSupportedException so failures are explicit, and replace the
whitespace-only tokenization with a configurable tokenizer/tokenization strategy
(inject ITokenizer or provide normalization/punctuation handling) so
tokenization is robust; also consider changing the class visibility to internal
if it should not be directly instantiated by consumers.
- Around line 170-197: ComputeContextHash and IsGreenToken currently call
string.GetHashCode() which is non-deterministic across processes; replace both
uses with a deterministic keyed hash (e.g., HMAC-SHA256) using the existing
_secretKey: compute an HMAC-SHA256 over each token (and for context,
sequentially update the HMAC with each token or concatenate tokens) to produce a
stable byte[] and convert a portion (e.g., first 4 bytes as unsigned int) into
an int for hashing/combining; in ComputeContextHash use the HMAC-derived ints
instead of tokens[i].GetHashCode() and drop the manual secret mixing loop, and
in IsGreenToken compute the token HMAC-derived int to XOR with the context hash
(or use HMAC of context+token) to produce the deterministic combined value used
to compute fraction against _greenListFraction.
---
Duplicate comments:
In `@src/Safety/SafetyConfig.cs`:
- Line 194: EffectiveLanguages currently allocates a new string[] on every
access; update the property on the SafetyConfig class so it returns a shared
default (e.g., a static readonly string[] or Array.Empty<string>()) instead of
new[] { "en" }, keeping the fallback when Languages is null; modify the
EffectiveLanguages getter to use that shared default to avoid repeated
allocations.
In `@src/Safety/SafetyPipelineFactory.cs`:
- Around line 28-35: Change the accessibility of the generic factory class to
hide internal plumbing: make SafetyPipelineFactory<T> internal instead of public
so only AiModelBuilder (and internal callers) can use it; ensure any tests or
internal references still compile (adjust visibility or add InternalsVisibleTo
if necessary) and leave AiModelBuilder and AiModelResult as the public facade
for users.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…voice protection, deepfake detection, integration tests Add remaining safety modules for issue #287: - Adversarial robustness: homoglyph, invisible char, leetspeak, mixed-script detection; image perturbation analysis - Fairness: demographic parity, equalized odds, stereotype detection - Multimodal: text-image alignment checker - Audio: acoustic toxicity, voiceprint/watermark deepfake detectors, voice protectors (masking, perturbation, watermark) - Image: ensemble classifier, deepfake detectors (frequency, consistency, provenance), ViT/scene-graph classifiers - Text: ensemble toxicity/jailbreak detectors, composite PII, hallucination detectors, copyright detectors - Video: multimodal video moderator with temporal+scene+motion analysis - Updated SafetyPipelineFactory to wire all 30+ modules - Added EvaluateImageSafety, EvaluateAudioSafety, EvaluateVideoSafety, EnforceSafetyPolicy to AiModelResult facade - Added Bias and TransparencyViolation to SafetyCategory enum - 40+ integration tests covering all safety subsystems Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 78 out of 78 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
- Make internal: CLIPImageSafetyClassifier, CrossModalConsistencyChecker, SpectralDeepfakeDetector, EUAIActComplianceChecker, SOC2ComplianceChecker, SafetyPipelineFactory (preserve facade pattern) - Add null guards: SafetyViolationException, TextSafetyModuleBase, VideoSafetyModuleBase, AudioSafetyModuleBase - SafetyReport: defensive copies, fix IsSafe logic for Modify action - SafetyPipeline: honor per-modality enable flags, integrate EffectiveDefaultAction and EffectiveMinimumActionSeverity in EnforcePolicy - SafetyConfig: cache DefaultLanguages array - PatternJailbreakDetector: fix inverted sensitivity thresholds, narrow Unicode tag detection to U+E0001-U+E007F only - TextWatermarker: replace hardcoded key with cryptographic random, replace string.GetHashCode with deterministic djb2 hash, implement vector Evaluate - CustomRuleGuardrail: specific catch clauses, sanitize error messages, implement vector Evaluate - OutputGuardrail: validate parameters, implement vector Evaluate - TopicRestrictionGuardrail: log RegexMatchTimeoutException - RuleBasedToxicityDetector: apply unused confidenceThreshold filter - GDPRComplianceChecker: remove empty if, implement vector Evaluate - CrossModalConsistencyChecker: use LINQ Where for override patterns - FrameSamplingVideoModerator: remove placeholder comment, unused var - TemporalConsistencyDetector: validate frame tensor sizes - SpectralDeepfakeDetector/TranscriptionToxicityDetector: extract IsZeroCrossing helper to simplify complex conditions - SafetyPipelineFactory: simplify multimodal condition with count, fix jailbreak sensitivity named parameter - SafetyBenchmarkRunner: validate test case text - StandardSafetyBenchmarks: cache benchmark arrays - ContextAwarePIIDetector: accept ITextSafetyModule interface - AiModelBuilder: attach safety pipeline on all build paths - AiModelResult: JsonIgnore on SafetyPipeline, validate text input Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ess, benchmarking, facade methods Adds remaining safety subsystems for issue #287: - Phase 5: Interfaces, base classes, configs, results for all detector types (text, image, audio, video, multimodal, watermarking, compliance) - Phase 6: Watermarking variants (lexical, sampling, syntactic, audio seal, spectral, invisible/neural/frequency image) - Phase 7: Fairness modules (representational bias, intersectional bias), guardrails (composite, rule-based), multimodal guardrail - Phase 8: Comprehensive benchmarking suite (toxicity, jailbreak, bias, PII, hallucination, watermark, adversarial, composite) - Phase 9: Adversarial robustness (ViT attack, adaptive randomized smoothing, prompt defense, preference alignment) - AiModelResult facade: 9 new convenience methods (GetSafetyReport, IsSafeOutput, ValidateInputSafety, ValidateOutputSafety, DetectHallucinations, DetectWatermark, DetectPII, EvaluateFairness, RunSafetyBenchmarks) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…les/AiDotNet into feature/safety-filtering-287
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 137 out of 172 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add parameter validation (threshold 0-1, null checks, positive values) to: EnsembleToxicityDetector, EnsembleJailbreakDetector, EnsembleImageSafetyClassifier, AdversarialImageEvaluator, AdversarialRobustnessEvaluator, AudioDeepfakeDetectorBase, VoiceProtectorBase, DeepfakeDetectorBase, IntersectionalBiasDetector, RepresentationalBiasDetector, ClassifierToxicityDetector, EmbeddingToxicityDetector, JailbreakDetectorBase, CopyrightDetectorBase, ViTAdversarialAttack, ComplianceModuleBase - Fix RuleBasedToxicityDetector excerpt using original text instead of lowercased - Fix GuardrailRule: skip empty patterns, catch specific exceptions in regex/custom - Fix ViTAdversarialAttack: throw NotSupportedException for unsupported input types - Remove Debug.WriteLine from TopicRestrictionGuardrail (library code) - Remove redundant using directives from ComplianceModuleBase, DeepfakeDetectorBase - Fix typo: ComputeFeeSqueezingScore -> ComputeFeatureSqueezingScore - Use SafetyCategory keys in IImageSafetyClassifier and ImageSafetyResult - Rename NSFWThreshold/CSAMDetection to NsfwThreshold/CsamDetection in ImageSafetyConfig - Replace magic numbers with named constants in GuardrailConfig - Cache BiasConfig.EffectiveProtectedAttributes to avoid repeated allocation - Make CategoryClassifier and ToxicConcept readonly structs - Separate CopyrightMatch into its own file from CopyrightResult - Fix HallucinationDetectorBase to use referenceText parameter and trim claims - Add null guard on VoiceProtectorBase.Evaluate content parameter - Add VoiceProtectorConfig effective property validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- SafetyBenchmarkRunner: restructure if-else chain to nested ifs - SpectralDeepfakeDetector: extract weighted terms into named variables - TranscriptionToxicityDetector: extract weighted terms into named variables - CustomRuleGuardrail: consolidate catch blocks with exception filter - CrossModalConsistencyChecker: replace foreach+if with LINQ Max - SafetyPipelineFactory: extract modality checks into helper methods - PatternJailbreakDetector: avoid double regex evaluation with Match Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 24
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/AdversarialRobustness/Attacks/ViTAdversarialAttack.cs`:
- Around line 107-110: The patch-count logic in ViTAdversarialAttack currently
floors the division and drops any trailing elements; change the computation to
round up so remainder elements form an extra patch—e.g., compute numPatches with
integer ceiling (use (length + patchElements - 1) / patchElements) or
Math.Ceiling((double)length / patchElements) and ensure patchElements != 0;
update the code around the existing variables _patchSize, patchElements, and
numPatches so all elements are covered.
In `@src/AdversarialRobustness/Defenses/AdversarialPreferenceAlignment.cs`:
- Around line 515-546: The EvaluatePrincipleCompliance method currently
implements a variance-based heuristic (using output vector variance and keyword
checks for "harm"/"safe" to set strictness) and should be documented as a
baseline implementation: add an XML documentation comment on the
EvaluatePrincipleCompliance method that states it uses a purely statistical
variance heuristic, explains the role of the 'principle' parameter and the
strictness thresholds, warns this is not semantic compliance, and notes a TODO
to replace or augment with semantic analysis in future iterations (reference
method name EvaluatePrincipleCompliance and the 'principle' parameter in the
doc).
- Around line 599-608: The code currently clamps adjusted outputs to [0,1] using
MathHelper.Clamp in the block that computes adjusted[] (uses variables adjusted,
adjustment, _klCoefficient, reward, NumOps and returns new Vector<T>), which
incorrectly assumes model outputs are normalized; change this by making the
clamp bounds configurable (e.g., add configurable fields/properties like
_outputMin and _outputMax or obtain the base model's output range from
_baseModel) and replace the hardcoded 0.0/1.0 with those bounds, or remove the
clamp entirely if you want to preserve the original output scale—ensure any
added fields have sensible defaults and are used where MathHelper.Clamp(val +
adjustment, ...) is currently called.
- Around line 311-318: The unchecked cast to IParameterizable<T, Vector<T>,
Vector<T>> is unsafe and can throw if baseModel implements IGradientComputable
but not IParameterizable; change the guard so you only cast when both interfaces
are present (e.g., check baseModel is IGradientComputable<T,Vector<T>,Vector<T>>
trainableModel && baseModel is IParameterizable<T,Vector<T>,Vector<T>>
parameterizable) and then call parameterizable.GetParameters() and CopyVector;
if the model lacks IParameterizable, skip saving referenceParams or handle that
branch safely rather than performing an unchecked cast.
- Around line 226-232: The AverageSeverity is incorrectly set equal to
SuccessRate; change it to compute the mean severity of successful attacks
instead of the count ratio: collect severity values for each attack (e.g., a
severityScores list or running severitySum) inside the routine that builds
adversarialPrompts/successArray, then set AverageSeverity = successfulAttacks >
0 ? severitySum / successfulAttacks : 0 (or use severityScores.Average()) when
constructing the RedTeamingResults<T>; keep SuccessRate as
(double)successfulAttacks / totalAttempts and ensure you reference
AverageSeverity, SuccessRate, successfulAttacks, totalAttempts,
adversarialPrompts and successArray in your fixes.
In `@src/AdversarialRobustness/Defenses/AdversarialPromptDefense.cs`:
- Around line 276-280: SaveModel and LoadModel accept external filePath but
don't validate it; add input validation in both methods (SaveModel and
LoadModel) to guard against null and empty/whitespace strings: throw
ArgumentNullException when filePath is null and throw ArgumentException (with a
clear message) when filePath is empty or only whitespace before calling
File.WriteAllBytes/File.ReadAllBytes; ensure exception types/messages follow
project conventions so callers get a clear, immediate error rather than a
downstream IO exception.
- Around line 243-258: Deserialize currently rehydrates prompt vector without
validating length, which can cause out-of-range errors when ApplyPromptToInput
uses the readonly _promptLength; update Deserialize (method Deserialize and
references to state.PromptData/_defensePrompt/_options) to validate that
state.PromptData is non-null and that state.PromptData.Length == _promptLength
(or handle allowed mismatch by rejecting/deserializing defensively), and if it
does not match throw an informative exception (ArgumentException or
InvalidOperationException) before constructing _defensePrompt so
ApplyPromptToInput never encounters mismatched lengths.
In `@src/Safety/Audio/PerturbationVoiceProtector.cs`:
- Around line 160-204: DetectPerturbationArtifacts currently only checks the
first few adjacent frame pairs (numPairs = Math.Min(4,...)), which misses
artifacts later in longer audio; change numPairs to use a configurable max
(e.g., maxPairs) and sample frame-pair start positions evenly across the entire
audio buffer instead of always starting at the beginning. Specifically, in
DetectPerturbationArtifacts replace the fixed p*frameSize indexing with evenly
distributed starts (compute stride or use interpolation: start1 =
0..(audio.Length - frameSize*2) mapped across p in [0,numPairs-1], ensure safe
casting and bounds checks and handle numPairs==1 fallback), keep the existing
spectral comparison logic (frameSize, lowBin/highBin, _fft.Forward,
diffSum/magSum) unchanged.
- Around line 49-54: The PerturbationVoiceProtector constructor lacks validation
for the perturbationStrength parameter; add a guard in the
PerturbationVoiceProtector(double perturbationStrength = 0.02, int sampleRate =
16000) constructor to ensure perturbationStrength is > 0 and < 1 (per the XML
doc) before calling NumOps.FromDouble, and throw an ArgumentOutOfRangeException
with a clear message if the value is outside that range so _perturbationStrength
is never set to invalid values.
In `@src/Safety/Fairness/EqualizedOddsChecker.cs`:
- Around line 128-132: The Evaluate(Vector<T> content) method in
EqualizedOddsChecker is a stub that silently returns no findings; replace it
with a fail-fast behavior or real evaluation: either implement the proper
vector-based fairness checks or, minimally, throw a clear exception (e.g.,
NotSupportedException or InvalidOperationException) with a message like
"Vector<T> inputs are not supported by EqualizedOddsChecker" so safety is not
silently bypassed; update the Evaluate(Vector<T> content) method body
accordingly and ensure callers handle the exception or route to the supported
Evaluate overload.
- Around line 68-75: The constructor EqualizedOddsChecker(double
disparityThreshold = 0.3) currently accepts any double; add explicit validation
in that constructor to ensure disparityThreshold is between 0 and 1 inclusive
and prevent always-on/always-off behavior: if disparityThreshold < 0 ||
disparityThreshold > 1 throw an ArgumentOutOfRangeException with
nameof(disparityThreshold) and a clear message, otherwise assign to the
_disparityThreshold field as before.
- Around line 139-145: The current loop in EqualizedOddsChecker.cs (iterating
words[] and comparing against groupTerms) uses word.Contains(term) which causes
false positives like "human" matching "man"; change the matching to normalized
whole-word matching: normalize both the token and group term (e.g., trim
punctuation, lower-case) and replace the Contains check with either
string.Equals(token, term, StringComparison.OrdinalIgnoreCase) or a
regex/word-boundary match (e.g., Regex.IsMatch(token,
$@"\b{Regex.Escape(term)}\b", RegexOptions.IgnoreCase)) in the loop that sets
isGroupTerm so only exact whole-word matches are considered. Ensure any
trimming/normalization is applied consistently to words[] and groupTerms before
comparison.
In `@src/Safety/Guardrails/GuardrailRule.cs`:
- Around line 162-178: The EvaluateMaxLength and EvaluateMinLength methods
silently return false when Patterns[0] is non-numeric or negative; instead
validate and fail fast at configuration time: in the GuardrailRule constructor
or a dedicated validator (e.g., ValidatePatterns or the class initializer) parse
Patterns[0] for the MaxLength/MinLength rule, ensure it is an integer >= 0, and
throw an ArgumentException with a clear message referencing the rule and the
invalid pattern if parsing/validation fails; after moving validation to
construction, change EvaluateMaxLength/EvaluateMinLength to use the validated
integer fields (e.g., _maxLength/_minLength) and remove the silent
parse/return-false logic.
In `@src/Safety/Image/ImageSafetyClassifierBase.cs`:
- Around line 10-13: The XML remarks for ImageSafetyClassifierBase are
inaccurate: remove the claim about "common image feature extraction utilities"
and tighten the summary to describe only the actual responsibilities (threshold
configuration and category scoring) of ImageSafetyClassifierBase; update the
<summary>/<para> to mention threshold configuration, category score
aggregation/scoring behavior, and that concrete subclasses implement the actual
classification algorithm (e.g., CLIP/ViT/ensemble) so readers look to derived
classes for feature extraction/implementation details.
In `@src/Safety/SafetyPipelineBuilder.cs`:
- Around line 66-70: Add a null-check in
SafetyPipelineBuilder<T>.AddModule(ISafetyModule<T> module) to fail fast when a
null module is passed: validate the module parameter and throw an
ArgumentNullException (or similar) before calling _modules.Add(module). This
prevents silent insertion of null into the _modules collection and avoids
downstream failures in Build() when iterating modules; reference AddModule,
ISafetyModule<T>, _modules and Build() when making the change.
- Around line 77-81: Add a null-check and element validation to
SafetyPipelineBuilder<T>.AddModules: ensure the incoming modules enumerable is
not null and throw an ArgumentNullException naming the parameter if it is, then
iterate the collection to validate no element is null (throw ArgumentException
or ArgumentNullException describing the offending parameter) before calling
_modules.AddRange(modules); mirror the behavior used in WithConfig for clear,
consistent parameter validation so callers get meaningful errors instead of a
BCL ArgumentNullException from AddRange and to prevent null elements being added
to _modules.
- Around line 55-59: The Configure method calls the configure delegate without
validating it; add the same null-check used by WithConfig (e.g., Guard.NotNull
or equivalent) to validate the configure parameter before invoking it. Update
SafetyPipelineBuilder<T>.Configure to guard against a null configure (use the
project’s Guard.NotNull(configure, nameof(configure)) pattern or throw
ArgumentNullException) and then call configure(_config) and return this.
- Around line 88-125: The four modality-specific builder methods (AddTextModule,
AddImageModule, AddAudioModule, AddVideoModule) currently call
_modules.Add(module) without validating module; add the same null-check pattern
used in AddModule: if module is null throw new
ArgumentNullException(nameof(module)) (or equivalent), then call
_modules.Add(module) and return this to ensure consistent null validation across
all module-adding methods.
In `@src/Safety/Text/HallucinationDetectorBase.cs`:
- Around line 25-26: Add a null-check for the generatedText parameter in the
EvaluateAgainstReference method of HallucinationDetectorBase by calling the
project's guard helper (e.g., Guard.NotNull(generatedText,
nameof(generatedText))) at the start of the method so callers receive an
immediate, consistent null-safety error rather than relying on downstream
behavior in ExtractClaims; keep existing behavior for referenceText unchanged.
- Around line 25-60: The current EvaluateAgainstReference method mixes
reference-based results with the fallback EvaluateText call, causing
inconsistent behavior when findings.Count == 0; update EvaluateAgainstReference
so it either (A) returns the reference-based findings directly (i.e., return
findings, which may be empty) to preserve pure reference evaluation, or (B) if
hybrid evaluation is intended, call EvaluateText(generatedText) and merge/union
its results with the reference findings (deduplicating by Category/Description)
before returning; modify the logic around findings.Count > 0 ? findings :
EvaluateText(generatedText) accordingly in EvaluateAgainstReference and ensure
references to EvaluateText, ExtractClaims, findings, and ModuleName are used to
locate and implement the change.
- Around line 41-52: Replace the literal magic numbers used in
HallucinationDetectorBase (the "10" in the claim length check and the "80" used
for the truncation threshold) with clearly named constants—e.g., add private
const int MinClaimLength = 10 and private const int TruncationLength = 80 at the
class level—then update the check (claim.Length < MinClaimLength) and the
description/truncation logic (use TruncationLength instead of 80) so the intent
is explicit and tuning is centralized; keep the existing behavior and string
truncation logic but reference the new constants in the claim filtering and the
Description construction.
In `@src/Safety/Text/NgramCopyrightDetector.cs`:
- Around line 113-136: The current n-gram loop sets longestMatch and
longestMatchText incorrectly because it only records the n size when a single
n-gram matches; update the logic in the for (int n = 4; n <= 6; n++) block
(symbols: _copyrightedNgrams, longestMatch, longestMatchText, totalMatches,
totalNgrams) to stop claiming a verbatim passage length there — either remove
setting/using longestMatchText in that initial aggregation so the later
FindLongestConsecutiveMatch method (FindLongestConsecutiveMatch) is the single
source of truth, or replace the assignment with a call into the
consecutive-match computation to derive the actual contiguous matched substring
and assign longestMatchText from that result; ensure totalMatches/totalNgrams
still aggregate n-gram counts but do not misreport longestMatchText.
- Around line 54-56: The constructor parameter sourceNames on
Text/NgramCopyrightDetector is unused dead code; either remove the parameter and
its XML docs and update all call sites to stop passing it, or persist it by
adding a private readonly field (e.g. _sourceNames) assigned in the constructor
and surface it in the detector's output (e.g. include the names in the created
findings/metadata returned by methods like Detect/CreateFindings or by extending
CopyrightFinding to accept a sourceNames list). Update XML docs and any unit
tests/call sites accordingly so there are no unused parameters or missing
attribution data.
- Around line 61-68: The constructor NgramCopyrightDetector currently accepts a
threshold but doesn't validate it against the documented (0-1) range; add
validation in the NgramCopyrightDetector constructor to ensure threshold > 0 and
< 1 and throw an ArgumentOutOfRangeException (with a clear message referencing
"threshold") if it is out of range, leaving the rest of the assignments to
_threshold and _minNgramLength unchanged so callers get immediate, explicit
feedback for invalid values.
---
Duplicate comments:
In `@src/AdversarialRobustness/Defenses/AdaptiveRandomizedSmoothing.cs`:
- Around line 66-85: Validate the incoming CertifiedDefenseOptions<T> values in
the AdaptiveRandomizedSmoothing constructor: after Guard.NotNull(options) add
checks that options.NumSamples > 0 (throw ArgumentOutOfRangeException if not)
and that options.ConfidenceLevel is strictly between 0 and 1 (throw
ArgumentOutOfRangeException if not); reference the class/constructor
AdaptiveRandomizedSmoothing and the options properties NumSamples and
ConfidenceLevel (which are later assumed by CertifyPrediction and the
Clopper–Pearson/bounds calculations) so callers cannot pass invalid values that
would cause divide-by-zero or invalid interval computations.
- Around line 229-234: AdaptiveRandomizedSmoothing.Reset() currently has an
empty body which is unacceptable; implement Reset to restore the instance to its
initial state by reloading the original options used to construct the
AdaptiveRandomizedSmoothing object (reset any mutated option fields), reseed or
recreate the RNG used by CertifyPrediction (e.g., the private _rng/_random
field), and clear any cached per-input adaptive sigma or temporary state (cache
variables referenced in CertifyPrediction/ComputeAdaptiveSigma); alternatively,
if this class truly requires no reset, remove Reset() from the interface and
implementations (update interface and all implementers) instead of leaving an
empty method.
In `@src/Safety/Benchmarking/SafetyBenchmarkBase.cs`:
- Around line 43-51: Loop lacks validation for each testCase and its Text; add a
guard at the top of the foreach to skip null entries and blank text before
calling pipeline.EvaluateText. Concretely, inside the loop check if (testCase ==
null || string.IsNullOrWhiteSpace(testCase.Text)) { continue; } then call
pipeline.EvaluateText(testCase.Text) and keep the existing tp/fn/fp/tn logic
using the returned report.
In `@src/Safety/Benchmarking/SafetyBenchmarkRunner.cs`:
- Around line 75-80: The foreach over testCases in SafetyBenchmarkRunner
accesses testCase.Text without checking for null; update the loop to first guard
against null testCase (e.g., if (testCase == null) continue) before inspecting
testCase.Text so null entries won't cause a NullReferenceException; locate the
loop that iterates testCases and add the null-check directly before the existing
string.IsNullOrWhiteSpace(testCase.Text) check.
- Around line 189-304: This file contains multiple public types violating the
one-type-per-file rule; split each public class (SafetyBenchmarkRunner<T>,
SafetyBenchmarkCase, SafetyBenchmarkResult, SafetyBenchmarkCategoryResult) into
its own file with the same namespace, moving each class definition into a new
file named for the type, update any usings and references accordingly, and keep
accessibility and XML docs intact; ensure SafetyBenchmarkResult.CategoryResults
still initializes to a new Dictionary and any internal references from
SafetyBenchmarkRunner<T> to the other types are updated to the new files.
In `@src/Safety/Guardrails/GuardrailRule.cs`:
- Around line 139-154: The EvaluateRegex method currently returns false on
RegexMatchTimeoutException which allows adversarial inputs to bypass the
guardrail; change the catch in EvaluateRegex to treat a timeout as a match
(return true) so timeouts fail-closed, and add a diagnostic log entry (using the
project's logging mechanism) inside the catch referencing the pattern
(Patterns[0]) and the exception to aid troubleshooting; keep existing behavior
for empty Patterns and respect CaseInsensitive when constructing the regex
options.
In `@src/Safety/SafetyReport.cs`:
- Around line 129-157: In the foreach (var finding in findings) loop in
SafetyReport.cs, add a null-check to skip any null finding (e.g., if (finding ==
null) continue;) and before using finding.Confidence clamp it to [0,1] (e.g.,
use Math.Clamp or Math.Min/Max) and use the clamped value for computing
weightedScore and any other calculations; keep the existing
Severity/SafetySeverity handling and updates to highestSeverity,
strictestAction, categories, and minConfidenceWeightedScore but ensure you
operate only on non-null findings and on the clamped confidence value.
In `@src/Safety/Text/HallucinationDetectorBase.cs`:
- Around line 65-76: ExtractClaims currently trims sentences but returns empty
entries (e.g., "Hello. .World"), so update ExtractClaims to filter out
empty/whitespace-only strings after trimming (use string.IsNullOrWhiteSpace or
s.Length > 0) and return only non-empty claims; this keeps callers like
EvaluateAgainstReference from receiving blank claims and avoids relying on
downstream length checks.
- InputGuardrail: reorder null check before IsNullOrWhiteSpace to avoid null input being treated as empty-input violation - CompositePIIDetector: add null validation for detectors parameter - NgramCopyrightDetector: remove unused usings - SafetyPipelineBuilder: add null check for Configure delegate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 135 out of 190 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ess files - Fix ViT patch count to include trailing elements via ceiling division - Compute actual AverageSeverity based on reward scores (not duplicate of SuccessRate) - Replace unsafe cast with pattern match + throw for IParameterizable check - Validate serialized prompt length before rehydration in AdversarialPromptDefense - Add filePath validation in SaveModel/LoadModel with null and existence checks - Add perturbationStrength range validation in PerturbationVoiceProtector - Document HashInt as MurmurHash3 finalizer - Add disparityThreshold bounds validation in EqualizedOddsChecker - Replace stub Evaluate(Vector) with NotSupportedException - Fix substring matching to use exact word comparison (prevents "human" → "man") - Validate MaxLength/MinLength patterns in GuardrailRule (fail-fast vs silent skip) - Add null validation on all SafetyPipelineBuilder Add* methods - Add generatedText null validation in HallucinationDetectorBase - Fix inconsistent fallback logic when reference check finds no issues - Extract magic numbers to named constants in HallucinationDetectorBase - Store sourceNames and add threshold validation in NgramCopyrightDetector - Fix longest match tracking to use consecutive n-gram runs - Tighten ImageSafetyClassifierBase XML remarks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add copy constructors to 24 Options classes - Add parameterless constructors to 23 Options classes - Add "For Beginners" XML docs to 16 Options files - Add "For Beginners" XML docs to 94 model files - Fix LanguageIdentifierOptions copy constructor (remove child-class props) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 229 out of 292 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Replace new Random(42) with RandomHelper.CreateSeededRandom(42) in AdversarialPreferenceAlignment and 7 test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rename NgramOverlapAdjustment to NgramToWordCountAdjustment for clarity - Clarify GuardrailRule error messages to specify >= 0 instead of non-negative - Change SOC2, EUAIAct, CrossModal checkers from internal to public for consistency - Remove duplicated Clamp helper in VoiceProtectorConfig, use MathPolyfill.Clamp Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 229 out of 292 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…arning - AudioGenOptions: remove duplicate Seed assignment in copy constructor - EnsembleToxicityDetector/JailbreakDetector: validate and normalize weights - CompositePIIDetector: preserve overlapping findings of different categories - TopicRestrictionGuardrail: use PolicyViolation category instead of PromptInjection - IPIIDetector: document EndIndex as exclusive - CopyrightDetectorBase: note performance tradeoff for n-gram extraction - FederatedTrainerBase: fix copy/paste remarks text - NCCLCommunicationBackend: remove duplicate remarks block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merged
4 of 5 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConfigureSafety()facade method on AiModelBuilder with nested sub-configs for all safety domains (text, image, audio, video, watermarking, guardrails, fairness, compliance)Architecture
New Files (21)
Facade Integration
AiModelBuilder.ConfigureSafety(Action<SafetyConfig>)— single entry pointAiModelResult.SafetyPipeline— runtime access to pipelineAiModelResult.EvaluateTextSafety(string)— convenience methodAiModelResult.GetSafetyConfig()— config inspectionCloses #287 (Phase 1 + Phase 2 text modules — image, audio, video, watermarking, guardrails, fairness, compliance, benchmarking phases to follow)
Test plan
dotnet build)🤖 Generated with Claude Code
Summary by CodeRabbit