code static analysis - #48

• Lower CI coverage gate to 80% to unblock builds while adding new code.
• Add starter implementations for common static-analysis tasks (rule matching, AST calls, deps,
 dedupe).
• Include small inline example inputs to demonstrate intended usage.

graph TD
  CI["GitHub Actions CI"] --> Pytest["pytest + coverage"] --> AlgoPkg["Algorithms package"] --> SA["static_analysis"]
  SA --> Rule["rule_matcher.py"]
  SA --> AST["ast_tree.py"]
  SA --> Dep["package_dependency.py"]
  SA --> Dedup["deduplicate_security_findings.py"]

Loading

The following are alternative approaches to this PR:

1. Keep 90% coverage by excluding scaffolding from coverage

• ➕ Maintains a stricter quality gate while work is in progress
• ➕ Avoids incentivizing adding non-valuable tests just to hit a number
• ➖ Coverage exclusions can hide real gaps if left in place
• ➖ Requires maintaining omit rules and revisiting later

2. Keep 90% coverage by adding minimal tests for new modules

• ➕ Preserves the stronger gate without special-case config
• ➕ Encourages executable examples instead of inline demo code
• ➖ May add test overhead before APIs/behavior stabilize
• ➖ Tests could be brittle if the starter code is still changing

3. Move examples to docs/tests and keep modules import-safe

• ➕ Avoids side effects (e.g., printing) at import time
• ➕ Sets a clean pattern for future static-analysis utilities
• ➖ Slightly more up-front structure (docs/tests) than quick in-file samples

Recommendation: If the coverage threshold is being lowered temporarily, consider pairing it with either (a) coverage omission for the new scaffolding or (b) minimal smoke tests, to avoid permanently weakening the gate. Also, prefer moving the inline demo data/prints into tests or documentation so importing these modules stays side-effect free as the static-analysis utilities evolve.

Enhancement (4) +81 / -0

ast_tree.pyAdd AST Node model and stub for function-call search +12/-0

Add AST Node model and stub for function-call search

• Introduces a simple Node class representing an AST-like tree structure. Adds a placeholder find_calls(...) API intended to return function-call sites from a tree.

Algorithms/static_analysis/ast_tree.py

deduplicate_security_findings.pyAdd stub for deduplicating security findings with sample input +21/-0

Add stub for deduplicating security findings with sample input

• Adds a deduplicate(findings) stub and an inline example findings list showing the expected input shape. Intended as a starting point for consolidating duplicate findings.

Algorithms/static_analysis/deduplicate_security_findings.py

package_dependency.pyAdd stub for dependency-cycle detection with sample graph +13/-0

Add stub for dependency-cycle detection with sample graph

• Adds a find_dependency_cycle(deps) stub and an inline dependency map example. Intended as a starter for detecting cycles in package/module dependencies.

Algorithms/static_analysis/package_dependency.py

rule_matcher.pyAdd simple pattern-based rule matcher with demo rules +35/-0

Add simple pattern-based rule matcher with demo rules

• Implements find_violations(...) that scans code lines for rule patterns and emits structured violation records. Includes inline demo code/rules and prints the result for illustration.

Algorithms/static_analysis/rule_matcher.py

Other (1) +1 / -1

cicd.ymlLower pytest coverage fail-under threshold to 80% +1/-1

Lower pytest coverage fail-under threshold to 80%

• Adjusts the CI test step to fail only when coverage drops below 80% (previously 90%). This relaxes the quality gate while new code is introduced.

.github/workflows/cicd.yml

Description

find_violations() annotates rules as List[str] but indexes each rule as a dict
(rule["pattern"], rule["severity"]), so a caller passing a list of strings (per the signature)
will crash with TypeError. The module’s own example uses a list of dicts, confirming the signature
is inconsistent with actual usage.

Code

Algorithms/static_analysis/rule_matcher.py[R3-10]

+def find_violations(code: List[str], rules: List[str]):
+    """
+    This function takes a list of code lines and a list of rules, and returns a list of violations found in the code.
+    """
+    violations = []
+    for rule in rules:
+        pattern = rule["pattern"]
+        severity = rule["severity"]

Relevance

⭐⭐ Medium

No historical evidence found for enforcing type-hint correctness in this repo’s reviews.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The function signature claims rules are strings, but the implementation immediately treats each rule
as a dict; passing strings will therefore fail. The local example constructs rules as dicts,
demonstrating the annotation is wrong rather than the implementation.

Algorithms/static_analysis/rule_matcher.py[3-10]
Algorithms/static_analysis/rule_matcher.py[27-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Algorithms/static_analysis/rule_matcher.py::find_violations()` declares `rules: List[str]`, but treats each element as a mapping with `pattern` and `severity`. This creates an API contract mismatch and can cause runtime errors for any caller following the type hint.

### Issue Context
The module’s own `rules = [{"pattern": ..., "severity": ...}]` example shows the intended structure is a list of dict-like objects.

### Fix Focus Areas
- Algorithms/static_analysis/rule_matcher.py[3-19]
- Algorithms/static_analysis/rule_matcher.py[27-32]

### Suggested fix
- Update the signature to something like `rules: Sequence[Mapping[str, str]]` (or define a `TypedDict` for rule shape).
- Optionally add a small validation/guard for missing keys and raise a clear exception (e.g., `ValueError`) rather than a `KeyError`/`TypeError`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

New static-analysis modules add public functions whose bodies are only pass, so calling them
always returns None and provides no usable behavior. Since the repo packages Algorithms*, these
stubs are part of the distributed surface area and can be imported by consumers.

Code

Algorithms/static_analysis/ast_tree.py[R8-12]

+def find_calls(root: Node, function_name: str):
+    """
+    This function takes AST root and function name as input and returns all function calls in the AST.
+    """
+    pass

Relevance

⭐⭐ Medium

No prior review history about shipping pass stubs; only packaging context noted in PR #40.

PR-#40

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Each cited function is currently a no-op with pass. The packaging configuration includes
Algorithms*, meaning these modules are discoverable/installable as part of the project.

Algorithms/static_analysis/ast_tree.py[8-12]
Algorithms/static_analysis/deduplicate_security_findings.py[1-3]
Algorithms/static_analysis/package_dependency.py[3-4]
pyproject.toml[17-19]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Several newly added static analysis helpers are empty stubs (`pass`). This silently returns `None` and makes the APIs unusable.

### Issue Context
These modules live under `Algorithms/`, which is included by the packaging configuration.

### Fix Focus Areas
- Algorithms/static_analysis/ast_tree.py[8-12]
- Algorithms/static_analysis/deduplicate_security_findings.py[1-3]
- Algorithms/static_analysis/package_dependency.py[3-4]
- pyproject.toml[17-19]

### Suggested fix
- Either implement the functions, or replace `pass` with `raise NotImplementedError("...")` so failures are explicit.
- Add minimal unit tests for intended behavior, or relocate unfinished code to a non-packaged examples area if it’s not meant to be consumed as a library API.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

rule_matcher.py executes demo code and prints at module import time, which can pollute stdout/logs
and create unwanted side effects for any consumer importing the module. This makes the module harder
to use as a library component.

Code

Algorithms/static_analysis/rule_matcher.py[R21-34]

+code = [
+    "import os",
+    "password = input()",
+    "print(password)"
+]
+
+rules = [
+    {
+      "pattern": "password = input()",
+      "severity": "HIGH"
+    }
+]
+
+print(find_violations(code, rules))

Relevance

⭐⭐ Medium

No historical evidence found about banning import-time demo/print side effects requiring __main__
guard.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The cited lines define example code/rules and call print(...) at module scope, which will run
on import.

Algorithms/static_analysis/rule_matcher.py[21-34]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The module runs a demo (`print(find_violations(...))`) at import time.

### Issue Context
Import-time side effects are surprising for library modules and can interfere with test output/logging.

### Fix Focus Areas
- Algorithms/static_analysis/rule_matcher.py[21-34]

### Suggested fix
Move the demo code under:
```py
if __name__ == "__main__":
   ...
```
so importing the module does not execute it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

CI reduces the required coverage from 90% to 80%, allowing substantially more untested code to pass
without failing the pipeline. This weakens the repo’s regression protection at merge time.

Code

.github/workflows/cicd.yml[31]

+          pytest -q --cov=Algorithms --cov-report=term-missing --cov-fail-under=80

Relevance

⭐ Low

PR #47 intentionally lowered cov-fail-under to 80% (90% was “too strict/brittle”).

PR-#47
PR-#43

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The workflow command explicitly enforces a lower minimum coverage percentage than before, which
directly changes CI’s pass/fail criteria.

.github/workflows/cicd.yml[29-32]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The CI workflow lowers `--cov-fail-under` from 90 to 80, which reduces the effectiveness of the coverage gate.

### Issue Context
New modules were added under `Algorithms/static_analysis/`, and the workflow explicitly measures coverage for `Algorithms`.

### Fix Focus Areas
- .github/workflows/cicd.yml[29-32]

### Suggested fix
- Prefer restoring the previous `--cov-fail-under` value.
- If the new code is intentionally excluded (e.g., educational scaffolding), explicitly omit that directory from coverage (e.g., via pytest-cov omit settings) rather than lowering the global threshold.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools