Conversation
Greptile SummaryThis PR removes Claude-generated section-marker comments (e.g. Key changes:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[test_no_section_markers] --> B[find_section_markers]
B --> C[os.walk REPO_ROOT]
C --> D[Prune ignored dirs in-place]
D --> E[Visit files in dir]
E --> F{_should_scan?}
F -->|py / yml / yaml / Dockerfile| G[Read file line by line]
F -->|Other extensions| C
G --> H{Line matches pattern?}
H -->|SEPARATOR_LINE| I{Whitelisted?}
H -->|INLINE_SECTION| I
H -->|REGION_MARKER| I
H -->|No match| G
I -->|Yes| G
I -->|No| J[Append to violations]
J --> G
G --> C
C --> K{violations empty?}
K -->|Yes| L[Test passes]
K -->|No| M[AssertionError with report]
Last reviewed commit: eb643da |
| IGNORED_DIRS = { | ||
| ".venv", | ||
| "venv", | ||
| "__pycache__", | ||
| "node_modules", | ||
| ".git", | ||
| "dist", | ||
| "build", | ||
| ".egg-info", | ||
| ".tox", | ||
| # third-party vendored code | ||
| "gtsam", | ||
| } |
There was a problem hiding this comment.
.egg-info pattern won't match real egg-info directory names
".egg-info" is listed as a literal directory name to ignore, but actual egg-info directories are named like dimos.egg-info or mypackage.egg-info — never the bare string ".egg-info". Neither the dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] pruning nor _is_ignored_dir will ever match a real egg-info directory.
In practice this is harmless because egg-info directories don't contain .py / .yml / .yaml files that would be scanned, but the entry is misleading and the protection it implies doesn't work. Consider replacing it with an endswith check or using a glob-style pattern:
# In _should_scan or an _is_ignored_dir replacement:
if any(part.endswith(".egg-info") for part in dirpath.split(os.sep)):
continueOr simply remove the ".egg-info" entry from IGNORED_DIRS and add an endswith guard at the top of the scanning loop.
| for dirpath, dirnames, filenames in os.walk(REPO_ROOT): | ||
| # Prune ignored directories in-place | ||
| dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] | ||
|
|
||
| if _is_ignored_dir(dirpath): | ||
| continue |
There was a problem hiding this comment.
_is_ignored_dir check is unreachable dead code
The in-place dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] mutation on line 94 tells os.walk never to descend into ignored directories. As a result, after the very first iteration (where dirpath == REPO_ROOT), every subsequent dirpath is guaranteed to be a non-ignored directory. The _is_ignored_dir(dirpath) guard on line 96 will therefore never return True in practice — it only triggers if REPO_ROOT itself contains an ignored directory component in its absolute path, which is essentially impossible.
Consider removing the dead check to avoid misleading future readers:
| for dirpath, dirnames, filenames in os.walk(REPO_ROOT): | |
| # Prune ignored directories in-place | |
| dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] | |
| if _is_ignored_dir(dirpath): | |
| continue | |
| for dirpath, dirnames, filenames in os.walk(REPO_ROOT): | |
| # Prune ignored directories in-place | |
| dirnames[:] = [d for d in dirnames if d not in IGNORED_DIRS] | |
| rel_dir = os.path.relpath(dirpath, REPO_ROOT) |
Problem
Section markers are added by Claude. We don't need them. If a file is too complicated, it shouldn't use sections, it should be split into smaller files.
Closes DIM-XXX
Solution
Breaking Changes
None.
How to Test
None.
Contributor License Agreement