ENH: Implement intelligence Gate for API Veracity (De-hallucination)#36
ENH: Implement intelligence Gate for API Veracity (De-hallucination)#36samtuckerdavis merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis campaign adds an optional veracity verification gate to the import review pipeline: a VeracityPlugin contract, a Python implementation that AST-checks fenced code blocks for hallucinated attribute accesses, CLI/config wiring for ChangesVeracity Verification Campaign
sequenceDiagram
participant CLI as CLI (user)
participant Cmd as Review Command
participant Holistic as Holistic Import Flow
participant LangReg as Language Registry / LangConfig
participant PyPlugin as PythonVeracityPlugin
participant Env as Local Python env
CLI->>Cmd: parse args (--verify-veracity)
Cmd->>Holistic: call import_holistic_issues(verify_veracity=true)
Holistic->>LangReg: resolve language config for issue.lang
LangReg->>PyPlugin: provide veracity_plugin (if present)
Holistic->>PyPlugin: verify_suggestion(suggestion, project_root)
PyPlugin->>Env: find_spec / import module -> hasattr checks
Env-->>PyPlugin: attribute exists? / not found
PyPlugin-->>Holistic: return [] or [VeracityIssue...]
Holistic->>Holistic: skip or import issue based on results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
desloppify/intelligence/review/importing/holistic.py (1)
109-115:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
_validate_and_build_issuescall arity mismatch.Line 110-Line 113 passes an extra positional argument (
holistic_prompts). With the current helper signature indesloppify/intelligence/review/importing/holistic_issue_flow.py:58-90, this will raiseTypeErrorduring import and break the veracity campaign path.Suggested fix
review_issues, skipped, dismissed_concerns = _validate_and_build_issues( issues_list, - holistic_prompts, lang_name, verify_veracity=verify_veracity, project_root=project_root, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desloppify/intelligence/review/importing/holistic.py` around lines 109 - 115, The call to _validate_and_build_issues is passing an extra positional argument (holistic_prompts) that doesn't exist on the helper's signature in holistic_issue_flow.py; update the call site to match the helper by removing the extra positional argument and only passing issues_list, lang_name, and the keyword args verify_veracity=verify_veracity and project_root=project_root (i.e., call _validate_and_build_issues(issues_list, lang_name, verify_veracity=verify_veracity, project_root=project_root)) so the arity matches the function defined in holistic_issue_flow.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desloppify/languages/python/tests/test_py_veracity.py`:
- Around line 35-39: The test asserts the wrong module name: update the
assertion in test_py_veracity.py so it expects "os.path" (the module_path built
by verify_suggestion in desloppify/languages/python/veracity.py) instead of
"os"; change the assertion that references issues[0]["module"] to compare
against "os.path" to match the detector's output for suggestion
"os.path.this_is_not_a_real_method(...)".
In `@desloppify/languages/python/veracity.py`:
- Around line 41-76: The verifier currently never records import bindings so
aliasing and from-imports bypass checks; update _check_tree to first walk the
AST and build a mapping of local names to module paths for both ast.Import and
ast.ImportFrom (handle "import X as Y" and "from A import B as C" and map
name->full module path), then pass that mapping into _verify_attribute_call (or
make it accessible) so _verify_attribute_call uses the mapping to resolve
base_name to the real module_path instead of relying only on the raw base
identifier; ensure _verify_attribute_call uses the resolved module_path when
calling importlib.util.find_spec and when constructing attr_name/submodules
(preserve existing parts/attr_name logic but substitute resolved
base_name/module_path where appropriate).
- Around line 78-90: The code currently swallows all errors with "except
Exception: pass", hiding verifier failures; update the try/except in veracity.py
around the importlib.import_module call (the block referencing base_name,
module_path, attr_name, block) to catch only expected introspection/import
errors (e.g., ImportError, ModuleNotFoundError, AttributeError) and treat them
as verification failures by returning the same VeracityIssue/dict (include the
actual exception text in the "message" for debugging), and for any other
unexpected exception re-raise it so real bugs aren't hidden.
- Around line 73-82: The current allowlist after
importlib.util.find_spec(base_name) restricts attribute checks to a tiny set;
remove that set check and instead import the resolved module_path and verify the
attribute for any module whose spec exists. Concretely, in the block using
find_spec(base_name) and importlib.import_module(module_path), drop the "if
base_name in {...}" guard and always attempt to import module_path and then
check hasattr(module, attr_name) (using the existing symbols find_spec,
module_path, importlib.import_module, and attr_name) so non-allowlisted stdlib
or installed packages get validated too.
---
Outside diff comments:
In `@desloppify/intelligence/review/importing/holistic.py`:
- Around line 109-115: The call to _validate_and_build_issues is passing an
extra positional argument (holistic_prompts) that doesn't exist on the helper's
signature in holistic_issue_flow.py; update the call site to match the helper by
removing the extra positional argument and only passing issues_list, lang_name,
and the keyword args verify_veracity=verify_veracity and
project_root=project_root (i.e., call _validate_and_build_issues(issues_list,
lang_name, verify_veracity=verify_veracity, project_root=project_root)) so the
arity matches the function defined in holistic_issue_flow.py.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b7432398-cf04-4d90-8403-3fefdb008641
📒 Files selected for processing (14)
ISSUE_DEHALLUCINATION.mddesloppify/app/cli_support/parser_groups_admin_review_options_core.pydesloppify/app/commands/review/cmd.pydesloppify/app/commands/review/importing/flags.pydesloppify/app/commands/review/importing/parse.pydesloppify/app/skill_docs.pydesloppify/data/global/SKILL.mddesloppify/intelligence/review/importing/holistic.pydesloppify/intelligence/review/importing/holistic_issue_flow.pydesloppify/intelligence/review/veracity.pydesloppify/languages/_framework/base/types.pydesloppify/languages/python/__init__.pydesloppify/languages/python/tests/test_py_veracity.pydesloppify/languages/python/veracity.py
| def _check_tree(self, tree: ast.AST, block: str) -> list[VeracityIssue]: | ||
| """Inspect AST for potentially hallucinated calls.""" | ||
| issues: list[VeracityIssue] = [] | ||
|
|
||
| # Simple visitor to find attribute accesses | ||
| for node in ast.walk(tree): | ||
| if isinstance(node, ast.Attribute): | ||
| issue = self._verify_attribute_call(node, block) | ||
| if issue: | ||
| issues.append(issue) | ||
|
|
||
| return issues | ||
|
|
||
| def _verify_attribute_call(self, node: ast.Attribute, block: str) -> VeracityIssue | None: | ||
| """Check if an attribute exists on its base (if base is a known module).""" | ||
| # Resolve the full module/object path (e.g. 'os.path') | ||
| parts = [] | ||
| curr = node | ||
| while isinstance(curr, ast.Attribute): | ||
| parts.append(curr.attr) | ||
| curr = curr.value | ||
|
|
||
| if not isinstance(curr, ast.Name): | ||
| return None | ||
|
|
||
| base_name = curr.id | ||
| # parts is [method, submodule], so reverse it and join | ||
| attr_name = parts[0] | ||
| submodules = parts[1:][::-1] | ||
|
|
||
| module_path = ".".join([base_name] + submodules) | ||
|
|
||
| # Check if it's a likely stdlib or installed module | ||
| spec = importlib.util.find_spec(base_name) | ||
| if not spec: | ||
| return None |
There was a problem hiding this comment.
Resolve imported bindings before auditing attribute chains.
This investigation never records import / from ... import ... bindings, so import os as o; o.path.this_is_not_a_real_method() and from os import path; path.this_is_not_a_real_method() both bypass the gate. The campaign spec calls for extracting imported modules first, and this leaves a real false-negative path.
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 71-71: Consider [base_name, *submodules] instead of concatenation
Replace with [base_name, *submodules]
(RUF005)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desloppify/languages/python/veracity.py` around lines 41 - 76, The verifier
currently never records import bindings so aliasing and from-imports bypass
checks; update _check_tree to first walk the AST and build a mapping of local
names to module paths for both ast.Import and ast.ImportFrom (handle "import X
as Y" and "from A import B as C" and map name->full module path), then pass that
mapping into _verify_attribute_call (or make it accessible) so
_verify_attribute_call uses the mapping to resolve base_name to the real
module_path instead of relying only on the raw base identifier; ensure
_verify_attribute_call uses the resolved module_path when calling
importlib.util.find_spec and when constructing attr_name/submodules (preserve
existing parts/attr_name logic but substitute resolved base_name/module_path
where appropriate).
| # Check if it's a likely stdlib or installed module | ||
| spec = importlib.util.find_spec(base_name) | ||
| if not spec: | ||
| return None | ||
|
|
||
| try: | ||
| # Safer than full import: just check if it's a common slop target | ||
| if base_name in {"os", "sys", "pathlib", "json", "hashlib", "re"}: | ||
| module = importlib.import_module(module_path) | ||
| if not hasattr(module, attr_name): |
There was a problem hiding this comment.
The six-module allowlist misses most hallucinated APIs.
After find_spec(base_name) succeeds, Line 80 still limits verification to {os, sys, pathlib, json, hashlib, re}. urllib.parse.this_does_not_exist() and any coalition dependency outside that set are accepted untouched, so the new gate only covers a small slice of Python suggestions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desloppify/languages/python/veracity.py` around lines 73 - 82, The current
allowlist after importlib.util.find_spec(base_name) restricts attribute checks
to a tiny set; remove that set check and instead import the resolved module_path
and verify the attribute for any module whose spec exists. Concretely, in the
block using find_spec(base_name) and importlib.import_module(module_path), drop
the "if base_name in {...}" guard and always attempt to import module_path and
then check hasattr(module, attr_name) (using the existing symbols find_spec,
module_path, importlib.import_module, and attr_name) so non-allowlisted stdlib
or installed packages get validated too.
| try: | ||
| # Safer than full import: just check if it's a common slop target | ||
| if base_name in {"os", "sys", "pathlib", "json", "hashlib", "re"}: | ||
| module = importlib.import_module(module_path) | ||
| if not hasattr(module, attr_name): | ||
| return { | ||
| "method": attr_name, | ||
| "module": module_path, | ||
| "message": f"Hallucinated API detected: '{module_path}.{attr_name}' does not exist.", | ||
| "code_block": block | ||
| } | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Stop swallowing verifier failures with except Exception.
except Exception: pass turns resolver failures into silent approvals. For example, os.path.join.non_existent() resolves to os.path.join; import_module() raises, the exception is swallowed, and no VeracityIssue is emitted. Catch only expected import/introspection errors and treat them as failed verification, or re-raise unexpected bugs. As per coding guidelines, use specific exception handling, never catch-all blocks; AI suppresses errors and removes safety checks.
🧰 Tools
🪛 Ruff (0.15.12)
[error] 89-90: try-except-pass detected, consider logging the exception
(S110)
[warning] 89-89: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desloppify/languages/python/veracity.py` around lines 78 - 90, The code
currently swallows all errors with "except Exception: pass", hiding verifier
failures; update the try/except in veracity.py around the
importlib.import_module call (the block referencing base_name, module_path,
attr_name, block) to catch only expected introspection/import errors (e.g.,
ImportError, ModuleNotFoundError, AttributeError) and treat them as verification
failures by returning the same VeracityIssue/dict (include the actual exception
text in the "message" for debugging), and for any other unexpected exception
re-raise it so real bugs aren't hidden.
…L.md
Three CI fixes:
1. Move VeracityPlugin/VeracityIssue to desloppify/intelligence/veracity.py
- Importing from intelligence.review.veracity triggered intelligence/review/__init__
- That __init__ calls available_langs() → load_all() while Python's __init__.py
is still loading as a partial module, causing Python to never register
- New location (intelligence/veracity.py) has no __init__ that triggers load_all
- intelligence/review/veracity.py becomes a thin re-export shim
2. Sync docs/SKILL.md with desloppify/data/global/SKILL.md (byte-identical copy)
- skill-version 6 → 7
- Added --verify-veracity line
- Fixed colon placement on Batch output heading
3. Fix test_hallucinated_suggestion assertion
- os.path.this_is_not_a_real_method correctly reports module == "os.path"
- Test was expecting "os" which is the top-level package, not the submodule
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
samtuckerdavis
dismissed
coderabbitai[bot]’s stale review
Issues addressed: circular import fixed by moving abstract base to intelligence/veracity.py, docs/SKILL.md synced, test assertion corrected.
Adversarial ReviewVerdict: pass What the PR doesAdds a Adversarial checksCorrectness of the circular import fix: The root issue was that Scope of veracity checking: Limited to Flag is opt-in: SKILL.md sync: No regressions identified. |
There was a problem hiding this comment.
♻️ Duplicate comments (3)
desloppify/languages/python/veracity.py (3)
41-52:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGate bypassed by aliased and
from-imports — import binding map is missing.
_check_treewalks the AST without first building an import-alias map. Any suggestion usingimport os as o; o.path.non_existent()orfrom os import path; path.non_existent()reaches_verify_attribute_callwithbase_name = 'o'orbase_name = 'path', neither of which matches the real module, sofind_specreturnsNoneand the call silently passes. The campaign's de-hallucination gate covers only a fraction of real AI output patterns.The fix is to collect
ast.Importandast.ImportFrombindings in a first AST pass and pass the alias→module-path map into_verify_attribute_callbefore thefind_speccall.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desloppify/languages/python/veracity.py` around lines 41 - 52, The _check_tree function currently walks the AST and calls _verify_attribute_call directly, which misses aliased imports and from-imports; first scan the tree for ast.Import and ast.ImportFrom nodes to build an alias→module-path binding map (capturing aliases like "import X as Y" and "from A import B as C" and mapping C or Y back to A.B or A), then change the loop in _check_tree to pass that binding map into _verify_attribute_call (and update _verify_attribute_call to accept the map) so that when it calls find_spec it resolves aliased/base names correctly instead of returning None.
78-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
except Exception: passviolates the coalition's specific-exception-handling rule.Any
ImportError,ModuleNotFoundError,AttributeError, or unexpected bug in the introspection path is swallowed and silently converted into a passing verdict.os.path.join.non_existent()for example causesimportlib.import_module('os.path.join')to raise, gets caught here, and the hallucination is approved without issue.Catch only the expected subset (
ImportError, AttributeError), return aVeracityIssuefor those (so the failed check surfaces), and re-raise anything else.🔧 Proposed fix
- except Exception: - pass + except (ImportError, AttributeError) as exc: + return { + "method": attr_name, + "module": module_path, + "message": f"Could not verify '{module_path}.{attr_name}': {exc}", + "code_block": block, + }As per coding guidelines,
desloppify/**/*.pymust "Use specific exception handling, never catch-all blocks; AI suppresses errors and removes safety checks."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desloppify/languages/python/veracity.py` around lines 78 - 90, The try/except is catching all exceptions and silently passing; change it to catch only ImportError, ModuleNotFoundError, and AttributeError for the introspection around base_name/module_path/attr_name and return a VeracityIssue (including message, method/module, code_block from block, and error details) when those expected failures occur, and re-raise any other unexpected exceptions so they aren't swallowed. Ensure the new except block references base_name, module_path, attr_name and block and constructs/returns a VeracityIssue (instead of silently passing) while leaving other errors to propagate.
73-82:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSix-module allowlist makes the gate nearly inert.
urllib.parse.non_existent(),requests.non_existent(),pytest.non_existent(), and every coalition dependency outside{"os", "sys", "pathlib", "json", "hashlib", "re"}are accepted without inspection. The campaign spec calls for verification against the local environment for any installed package.Drop the
if base_name in {...}guard:find_specalready confirms the module is resolvable, so just attemptimport_module(module_path)for any module with a valid spec.🔧 Proposed fix
- try: - # Safer than full import: just check if it's a common slop target - if base_name in {"os", "sys", "pathlib", "json", "hashlib", "re"}: - module = importlib.import_module(module_path) - if not hasattr(module, attr_name): - return { - "method": attr_name, - "module": module_path, - "message": f"Hallucinated API detected: '{module_path}.{attr_name}' does not exist.", - "code_block": block - } - except Exception: - pass + try: + module = importlib.import_module(module_path) + except (ImportError, ModuleNotFoundError) as exc: + return { + "method": attr_name, + "module": module_path, + "message": f"Could not import '{module_path}': {exc}", + "code_block": block, + } + try: + if not hasattr(module, attr_name): + return { + "method": attr_name, + "module": module_path, + "message": f"Hallucinated API detected: '{module_path}.{attr_name}' does not exist.", + "code_block": block, + } + except AttributeError as exc: + return { + "method": attr_name, + "module": module_path, + "message": f"Could not verify '{module_path}.{attr_name}': {exc}", + "code_block": block, + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desloppify/languages/python/veracity.py` around lines 73 - 82, The current guard limiting imports to {"os","sys","pathlib","json","hashlib","re"} makes verification inert; after confirming a spec via importlib.util.find_spec(base_name) simply call importlib.import_module(module_path) for any resolvable module (remove the if base_name in {...} branch) and then check hasattr(module, attr_name) as before; ensure the existing try/except around importlib.import_module stays in place so import errors are handled and return None when the attribute is absent or import fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@desloppify/languages/python/veracity.py`:
- Around line 41-52: The _check_tree function currently walks the AST and calls
_verify_attribute_call directly, which misses aliased imports and from-imports;
first scan the tree for ast.Import and ast.ImportFrom nodes to build an
alias→module-path binding map (capturing aliases like "import X as Y" and "from
A import B as C" and mapping C or Y back to A.B or A), then change the loop in
_check_tree to pass that binding map into _verify_attribute_call (and update
_verify_attribute_call to accept the map) so that when it calls find_spec it
resolves aliased/base names correctly instead of returning None.
- Around line 78-90: The try/except is catching all exceptions and silently
passing; change it to catch only ImportError, ModuleNotFoundError, and
AttributeError for the introspection around base_name/module_path/attr_name and
return a VeracityIssue (including message, method/module, code_block from block,
and error details) when those expected failures occur, and re-raise any other
unexpected exceptions so they aren't swallowed. Ensure the new except block
references base_name, module_path, attr_name and block and constructs/returns a
VeracityIssue (instead of silently passing) while leaving other errors to
propagate.
- Around line 73-82: The current guard limiting imports to
{"os","sys","pathlib","json","hashlib","re"} makes verification inert; after
confirming a spec via importlib.util.find_spec(base_name) simply call
importlib.import_module(module_path) for any resolvable module (remove the if
base_name in {...} branch) and then check hasattr(module, attr_name) as before;
ensure the existing try/except around importlib.import_module stays in place so
import errors are handled and return None when the attribute is absent or import
fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48953107-f16f-4335-bb50-c881b98fdd75
📒 Files selected for processing (6)
desloppify/intelligence/review/veracity.pydesloppify/intelligence/veracity.pydesloppify/languages/_framework/base/types.pydesloppify/languages/python/tests/test_py_veracity.pydesloppify/languages/python/veracity.pydocs/SKILL.md
2 participants
ISSUE: Implement
intelligenceGate for API Veracity (De-hallucination)Goal
Prevent AI agents from proposing "slop" fixes that utilize hallucinated library methods or deprecated APIs. This is a common failure mode where agents invent methods that "should" exist but do not.
Context
desloppifyintelligence/review/importing/holistic.py(specificallyimport_holistic_issues).Specification
ReviewIssuePayloadduring the import process.suggestionfield.sys.modules,pkg_resources, or by inspecting the AST of installed packages).desloppify/languages/python/detectors/deps_resolution.pyif applicable.VerificationIssueto the agent with a clear message:"Hallucinated API detected: [method_name]. Please verify against the actual library structure and refactor."--verify-veracity.Definition of Done
os.path.non_existent_method()is rejected.os.path.exists()) are accepted.skill_docs.py.Summary by CodeRabbit
New Features
Documentation
Tests