-
Notifications
You must be signed in to change notification settings - Fork 55
fix: support JSONC comments and monorepo tsconfig.json discovery #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,10 +153,15 @@ def _assess_python_types(self, repository: Repository) -> Finding: | |
| ) | ||
|
|
||
| def _assess_typescript_types(self, repository: Repository) -> Finding: | ||
| """Assess TypeScript type configuration.""" | ||
| tsconfig_path = repository.path / "tsconfig.json" | ||
| """Assess TypeScript type configuration across all tsconfig.json files. | ||
|
|
||
| if not tsconfig_path.exists(): | ||
| Supports monorepos with per-package tsconfig.json and JSONC comments. | ||
| """ | ||
| import json | ||
|
|
||
| tsconfig_files = self._find_tsconfig_files(repository) | ||
|
|
||
| if not tsconfig_files: | ||
| return Finding( | ||
| attribute=self.attribute, | ||
| status="fail", | ||
|
|
@@ -168,41 +173,45 @@ def _assess_typescript_types(self, repository: Repository) -> Finding: | |
| error_message=None, | ||
| ) | ||
|
|
||
| try: | ||
| import json | ||
| strict_count = 0 | ||
| total_count = 0 | ||
| evidence: list[str] = [] | ||
|
|
||
| with open(tsconfig_path, "r") as f: | ||
| tsconfig = json.load(f) | ||
| for tsconfig_path in tsconfig_files: | ||
| rel_path = str(tsconfig_path.relative_to(repository.path)) | ||
| total_count += 1 | ||
| try: | ||
| raw = tsconfig_path.read_text(encoding="utf-8") | ||
| cleaned = self._strip_json_comments(raw) | ||
| tsconfig = json.loads(cleaned) | ||
| except (OSError, json.JSONDecodeError) as e: | ||
| evidence.append(f"{rel_path}: parse error ({e})") | ||
| continue | ||
|
|
||
| strict = tsconfig.get("compilerOptions", {}).get("strict", False) | ||
|
|
||
| if strict: | ||
| return Finding( | ||
| attribute=self.attribute, | ||
| status="pass", | ||
| score=100.0, | ||
| measured_value="strict mode enabled", | ||
| threshold="strict mode enabled", | ||
| evidence=["tsconfig.json has strict: true"], | ||
| remediation=None, | ||
| error_message=None, | ||
| ) | ||
| strict_count += 1 | ||
| evidence.append(f"{rel_path}: strict: true") | ||
| else: | ||
| return Finding( | ||
| attribute=self.attribute, | ||
| status="fail", | ||
| score=50.0, | ||
| measured_value="strict mode disabled", | ||
| threshold="strict mode enabled", | ||
| evidence=["tsconfig.json missing strict: true"], | ||
| remediation=self._create_remediation(), | ||
| error_message=None, | ||
| ) | ||
| evidence.append(f"{rel_path}: strict mode disabled") | ||
|
Comment on lines
+180
to
+196
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "TypeScript assessor logic:"
sed -n '181,197p' src/agentready/assessors/code_quality.py
echo
echo 'Search for any tsconfig "extends" handling in the assessor and tests:'
rg -n --type=py '\bextends\b|compilerOptions.*strict|_strip_json_comments|_find_tsconfig_files' \
src/agentready/assessors/code_quality.py \
tests/unit/test_assessors_typescript.pyRepository: ambient-code/agentready Length of output: 2357 Fix TypeScript strict scoring to account for
🤖 Prompt for AI Agents |
||
|
|
||
| except (OSError, json.JSONDecodeError) as e: | ||
| return Finding.error( | ||
| self.attribute, reason=f"Could not parse tsconfig.json: {str(e)}" | ||
| ) | ||
| score = self.calculate_proportional_score( | ||
| measured_value=(strict_count / total_count) * 100, | ||
| threshold=100.0, | ||
| higher_is_better=True, | ||
| ) | ||
| status = "pass" if strict_count == total_count else "fail" | ||
|
|
||
| return Finding( | ||
| attribute=self.attribute, | ||
| status=status, | ||
| score=score, | ||
| measured_value=f"{strict_count}/{total_count} strict", | ||
| threshold="all tsconfig.json files strict", | ||
| evidence=evidence, | ||
| remediation=self._create_remediation() if status == "fail" else None, | ||
| error_message=None, | ||
| ) | ||
|
|
||
| @staticmethod | ||
| def _strip_go_non_code(content: str) -> str: | ||
|
|
@@ -270,6 +279,59 @@ def _strip_go_non_code(content: str) -> str: | |
|
|
||
| return "".join(out) | ||
|
|
||
| @staticmethod | ||
| def _strip_json_comments(text: str) -> str: | ||
| """Strip // and /* */ comments from JSONC, preserving string contents.""" | ||
| out: list[str] = [] | ||
| i = 0 | ||
| n = len(text) | ||
| while i < n: | ||
| c = text[i] | ||
|
|
||
| if c == '"': | ||
| out.append(c) | ||
| i += 1 | ||
| while i < n and text[i] != '"': | ||
| if text[i] == "\\" and i + 1 < n: | ||
| out.append(text[i]) | ||
| out.append(text[i + 1]) | ||
| i += 2 | ||
| else: | ||
| out.append(text[i]) | ||
| i += 1 | ||
| if i < n: | ||
| out.append(text[i]) | ||
| i += 1 | ||
| continue | ||
|
|
||
| if c == "/" and i + 1 < n and text[i + 1] == "/": | ||
| i += 2 | ||
| while i < n and text[i] != "\n": | ||
| i += 1 | ||
| continue | ||
|
|
||
| if c == "/" and i + 1 < n and text[i + 1] == "*": | ||
| i += 2 | ||
| while i + 1 < n and not (text[i] == "*" and text[i + 1] == "/"): | ||
| i += 1 | ||
| i += 2 | ||
| continue | ||
|
|
||
| out.append(c) | ||
| i += 1 | ||
|
|
||
| return "".join(out) | ||
|
|
||
| def _find_tsconfig_files(self, repository: Repository) -> list: | ||
| """Find all tsconfig.json files, excluding node_modules/vendor/testdata.""" | ||
| found = [] | ||
| for tsconfig in repository.path.rglob("tsconfig.json"): | ||
| parts = tsconfig.parts | ||
| if "node_modules" in parts or "vendor" in parts or "testdata" in parts: | ||
| continue | ||
| found.append(tsconfig) | ||
| return sorted(found) | ||
|
Comment on lines
+325
to
+333
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prune excluded directories during traversal. This still walks 🤖 Prompt for AI Agents |
||
|
|
||
| def _assess_go_types(self, repository: Repository) -> Finding: | ||
| """Assess Go type safety. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document that partial strict scores still fail.
This now reads like
2/3 strict = 67%is the user-facing outcome, but the assessor only returnspasswhen every discoveredtsconfig.jsonis strict and treats malformed configs as non-strict. Please add that 100% is required to pass so the docs match the implementation.As per coding guidelines, "Keep
docs/attributes.mdin sync when changing assessor scoring logic, thresholds, partial credit rules, recognized paths, or pass/fail conditions"🤖 Prompt for AI Agents