feat: add signal implementations to drift-engine package (ADR-100 phase 3a-signals)#715
Open
mick-gsk wants to merge 1 commit into
Open
feat: add signal implementations to drift-engine package (ADR-100 phase 3a-signals)#715mick-gsk wants to merge 1 commit into
mick-gsk wants to merge 1 commit into
Conversation
Owner
Author
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Phase 3a “signals” implementations to the drift-engine package as part of the ADR-100 monorepo migration decomposition, introducing multiple deterministic (AST-only / LLM-free) detectors and TS/JS rule integration.
Changes:
- Introduces a large set of new signal implementations (Python + TS/JS) producing
Findingobjects across multiple drift dimensions. - Adds shared signal infrastructure utilities (base types, TS tree-sitter helpers, dependency ordering).
- Packages
drift-engineas a standalone Python project (pyproject.toml) and wires signal exports.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/drift-engine/src/drift_engine/signals/type_safety_bypass.py | New TS/TSX detector for type-system bypass patterns. |
| packages/drift-engine/src/drift_engine/signals/ts_architecture.py | New signal that runs TS/JS architecture rule runners and emits findings. |
| packages/drift-engine/src/drift_engine/signals/test_polarity_deficit.py | New test polarity (“happy path only”) detector for Python + TS/JS tests. |
| packages/drift-engine/src/drift_engine/signals/temporal_volatility.py | New git-history-driven volatility/churn detector. |
| packages/drift-engine/src/drift_engine/signals/system_misalignment.py | New detector for novel dependency introduction vs historical baseline. |
| packages/drift-engine/src/drift_engine/signals/pattern_fragmentation.py | New detector for intra-module pattern variant fragmentation with dampeners. |
| packages/drift-engine/src/drift_engine/signals/missing_authorization.py | New detector for missing auth checks on inferred endpoints. |
| packages/drift-engine/src/drift_engine/signals/insecure_default.py | New detector for insecure config defaults (Python AST + TS regex). |
| packages/drift-engine/src/drift_engine/signals/guard_clause_deficit.py | New detector for missing guard clauses + deep nesting. |
| packages/drift-engine/src/drift_engine/signals/fan_out_explosion.py | New detector for excessive fan-out (import count). |
| packages/drift-engine/src/drift_engine/signals/explainability_deficit.py | New detector for high-complexity functions lacking doc/tests evidence. |
| packages/drift-engine/src/drift_engine/signals/exception_contract_drift.py | New git-history-driven detector for exception-profile contract drift. |
| packages/drift-engine/src/drift_engine/signals/dependency_dag.py | New signal dependency DAG ordering helper (topological sort + cache). |
| packages/drift-engine/src/drift_engine/signals/dead_code_accumulation.py | New detector for “exported but never imported” dead code with heuristics. |
| packages/drift-engine/src/drift_engine/signals/cohesion_deficit.py | New detector for semantic cohesion deficits via token overlap. |
| packages/drift-engine/src/drift_engine/signals/cognitive_complexity.py | New cognitive complexity detector (Python AST + TS tree-sitter). |
| packages/drift-engine/src/drift_engine/signals/co_change_coupling.py | New git-history-driven hidden coupling detector via co-change patterns. |
| packages/drift-engine/src/drift_engine/signals/circular_import.py | New Python circular import cycle detector. |
| packages/drift-engine/src/drift_engine/signals/bypass_accumulation.py | New marker-density detector for quality/type/lint bypass accumulation. |
| packages/drift-engine/src/drift_engine/signals/broad_exception_monoculture.py | New detector for monoculture broad exception catching + swallowing. |
| packages/drift-engine/src/drift_engine/signals/base.py | Adds signal base classes, registry, and caching dependency specs. |
| packages/drift-engine/src/drift_engine/signals/_utils.py | Adds shared signal utilities and supported-language constants. |
| packages/drift-engine/src/drift_engine/signals/_ts_support.py | Central TS tree-sitter parsing/walking helpers. |
| packages/drift-engine/src/drift_engine/signals/init.py | Exposes signal classes for package import/registration. |
| packages/drift-engine/src/drift_engine/init.py | Defines package-level documentation for drift-engine. |
| packages/drift-engine/pyproject.toml | Declares drift-engine project metadata and dependencies. |
Comments suppressed due to low confidence (1)
packages/drift-engine/src/drift_engine/signals/ts_architecture.py:1
- This helper is unused and its docstring/comment contradict its behavior (
return Noneunconditionally). Removing it (or converting it into a real implementation if needed) will reduce dead code and avoid confusion for future maintainers.
|
|
||
| file_name = pr.file_path.name | ||
| if file_name in _SKIP_FILES and ignore_re_exports: | ||
| continue |
Comment on lines
+70
to
+75
| if ( | ||
| isinstance(stmt, ast.If) | ||
| and not stmt.orelse | ||
| and any(isinstance(s, ast.Raise | ast.Return) for s in stmt.body) | ||
| ): | ||
| return _references_param(stmt.test, param_names) |
| return True # benefit of doubt | ||
|
|
||
| for node in ast.walk(tree): | ||
| if not isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef): |
Comment on lines
+3
to
+24
| from drift_engine.signals.architecture_violation import ArchitectureViolationSignal | ||
| from drift_engine.signals.base import BaseSignal | ||
| from drift_engine.signals.broad_exception_monoculture import BroadExceptionMonocultureSignal | ||
| from drift_engine.signals.bypass_accumulation import BypassAccumulationSignal | ||
| from drift_engine.signals.circular_import import CircularImportSignal | ||
| from drift_engine.signals.co_change_coupling import CoChangeCouplingSignal | ||
| from drift_engine.signals.cognitive_complexity import CognitiveComplexitySignal | ||
| from drift_engine.signals.cohesion_deficit import CohesionDeficitSignal | ||
| from drift_engine.signals.dead_code_accumulation import DeadCodeAccumulationSignal | ||
| from drift_engine.signals.doc_impl_drift import DocImplDriftSignal | ||
| from drift_engine.signals.exception_contract_drift import ExceptionContractDriftSignal | ||
| from drift_engine.signals.explainability_deficit import ExplainabilityDeficitSignal | ||
| from drift_engine.signals.fan_out_explosion import FanOutExplosionSignal | ||
| from drift_engine.signals.guard_clause_deficit import GuardClauseDeficitSignal | ||
| from drift_engine.signals.mutant_duplicates import MutantDuplicateSignal | ||
| from drift_engine.signals.naming_contract_violation import NamingContractViolationSignal | ||
| from drift_engine.signals.pattern_fragmentation import PatternFragmentationSignal | ||
| from drift_engine.signals.phantom_reference import PhantomReferenceSignal | ||
| from drift_engine.signals.system_misalignment import SystemMisalignmentSignal | ||
| from drift_engine.signals.temporal_volatility import TemporalVolatilitySignal | ||
| from drift_engine.signals.test_polarity_deficit import TestPolarityDeficitSignal | ||
| from drift_engine.signals.ts_architecture import TypeScriptArchitectureSignal |
Comment on lines
+26
to
+29
| cache_key = frozenset(classes) | ||
| cached = _topo_cache.get(cache_key) | ||
| if cached is not None: | ||
| return cached |
Comment on lines
+79
to
+81
| with _topo_cache_lock: | ||
| _topo_cache[cache_key] = ordered | ||
| return ordered |
Comment on lines
+279
to
+286
| try: | ||
| proc = subprocess.run( | ||
| ["git", "cat-file", "--batch"], | ||
| input=stdin_data.encode(), | ||
| capture_output=True, | ||
| cwd=str(repo_path), | ||
| timeout=30, | ||
| ) |
Comment on lines
+166
to
+170
| def visit_FunctionDef(self, node: ast.FunctionDef) -> None: # noqa: N802 | ||
| if node.name.startswith("test_") or node.name.startswith("test"): | ||
| # Finalise previous test function tracking | ||
| self._finalise_current_test() | ||
| self.test_functions += 1 |
Comment on lines
+69
to
+75
| for fn in pr.functions: | ||
| if fn.return_type and _ANY_ANNOTATION.search(fn.return_type): | ||
| count += 1 | ||
| for param in fn.parameters: | ||
| ptype = param.get("type", "") if isinstance(param, dict) else "" | ||
| if ptype and _ANY_ANNOTATION.search(ptype): | ||
| count += 1 |
| {"typescript", "tsx", "javascript", "jsx"}, | ||
| ) | ||
|
|
||
| _SUPPORTED_LANGUAGES: frozenset[str] = frozenset({"python"}) | _TS_LANGUAGES |
| class EmbeddingServiceProtocol(Protocol): | ||
| """Minimal interface expected by signal infrastructure for embeddings.""" | ||
|
|
||
| def embed_text(self, text: str) -> np.ndarray | None: ... |
|
|
||
| def embed_text(self, text: str) -> np.ndarray | None: ... | ||
|
|
||
| def embed_texts(self, texts: list[str]) -> list[np.ndarray | None]: ... |
| def embed_texts(self, texts: list[str]) -> list[np.ndarray | None]: ... | ||
|
|
||
| @staticmethod | ||
| def cosine_similarity(a: np.ndarray, b: np.ndarray) -> float: ... |
| def cosine_similarity(a: np.ndarray, b: np.ndarray) -> float: ... | ||
|
|
||
| @staticmethod | ||
| def build_index(vectors: list[np.ndarray] | np.ndarray) -> object | None: ... |
| all_checked.update( | ||
| _extract_functions_from_source(src).keys() | ||
| ) | ||
| except (OSError, UnicodeDecodeError): |
| SIMILARITY_THRESHOLD = 0.80 | ||
|
|
||
| # Hybrid similarity threshold (lower because embedding adds info) | ||
| _HYBRID_THRESHOLD = 0.75 |
|
|
||
|
|
||
| _TS_CHECKERS: dict[str, Callable[..., bool]] = { | ||
| "_has_rejection_path": lambda root, src, fn: _ts_has_rejection_path(root, src, fn), |
| "_has_rejection_path": lambda root, src, fn: _ts_has_rejection_path(root, src, fn), | ||
| "_has_raise": lambda root, src, _fn: _ts_has_raise(root, src), | ||
| "_has_create_path": lambda root, src, _fn: _ts_has_create_path(root, src), | ||
| "_has_bool_return": lambda root, src, fn: _ts_has_bool_return(root, src, fn), |
| _STDLIB_MODULES: frozenset[str] = frozenset(sys.stdlib_module_names) | ||
|
|
||
| # Minimum function count to flag a file (avoids noise on tiny scripts) | ||
| _MIN_CALLS_FOR_FINDING = 1 |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Split from #576 (ADR-100 monorepo migration). Part of the PR decomposition into atomic, reviewable units.