Fix tasks subsystem parity coverage regression on top of PR #154#166
Fix tasks subsystem parity coverage regression on top of PR #154#166badMade wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the ARCHIVE_DIR_MAPPINGS in src/parity_audit.py to map the tasks subsystem to a directory instead of a single file. The review feedback highlights that the current audit logic only performs top-level directory checks, which may lead to inaccurate coverage reporting now that tasks is a directory; it is recommended to update the logic to support recursive checks. Additionally, the reviewer suggests evaluating whether other subsystems should also be transitioned to directory mappings for consistency.
| 'skills': 'skills', | ||
| 'state': 'state', | ||
| 'tasks': 'task.py', | ||
| 'tasks': 'tasks', |
There was a problem hiding this comment.
The current audit logic in run_parity_audit (lines 122-124) only checks for existence within the top-level src/ directory using iterdir(). As subsystems like tasks transition to directory structures, any files moved into those directories (e.g., moving task.py into tasks/) will no longer be detected by the audit, causing a drop in reported root file coverage. Consider updating the audit logic to support recursive checks or relative path mappings to ensure continued accuracy as the project structure evolves.
There was a problem hiding this comment.
@gemini-code-assist, what are the steps and line numbers to make this fix?
There was a problem hiding this comment.
To resolve the parity coverage regression, you need to update the logic within the run_parity_audit function in src/parity_audit.py. Currently, the audit likely uses a check that only validates file existence (such as os.path.isfile or a direct check on a file path).
To support directory structures, you should update the validation logic to use pathlib.Path.exists(), which returns True for both files and directories.
Steps:
- Locate the
run_parity_auditfunction (typically around line 122). - Identify the loop iterating over
ARCHIVE_DIR_MAPPINGS. - Replace the existing file-specific check with
pathlib.Path(target_path).exists(). This will ensure that whentasksis mapped to thetasks/directory, the audit correctly identifies it as present.
| 'skills': 'skills', | ||
| 'state': 'state', | ||
| 'tasks': 'task.py', | ||
| 'tasks': 'tasks', |
There was a problem hiding this comment.
Consider whether other subsystems currently mapped to .py files (such as commands, context, ink, query, and tools) should also be updated to map to directories. Maintaining consistency across the ARCHIVE_DIR_MAPPINGS ensures that missing parity is not masked for these subsystems if they are intended to follow the same structural pattern as tasks.
There was a problem hiding this comment.
@gemini-code-assist, what are the steps and line numbers to make this fix?
There was a problem hiding this comment.
To determine if these mappings should be updated, you should verify if a corresponding directory exists for each of these subsystems within the src/ folder. The ARCHIVE_DIR_MAPPINGS dictionary in src/parity_audit.py is intended to track directory-based parity; if a subsystem is strictly file-based (e.g., src/tools.py exists but src/tools/ does not), it should remain mapped to the .py file.
If you find that a directory exists for any of these, you can update the mapping by changing the value from the filename to the directory name. Based on the current structure of src/parity_audit.py, you can find these mappings in the ARCHIVE_DIR_MAPPINGS dictionary (around lines 40-70). For example, if you were to update tools:
- Locate the line:
'tools': 'tools.py', - Change it to:
'tools': 'tools',
If these subsystems do not have corresponding directories, no changes are required.
There was a problem hiding this comment.
Pull request overview
This PR fixes a parity-audit regression where src/parity_audit.py incorrectly treated task.py as satisfying parity for the archived tasks/ directory, masking that the Python port is missing the tasks directory surface.
Changes:
- Update
ARCHIVE_DIR_MAPPINGS['tasks']to map to'tasks'(a directory) instead of'task.py'(a file), so directory parity is tracked independently of the roottasks.ts -> task.pyshim mapping.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@copilot, resolve the merge conflicts in this pull request. |
|
@badMade Unfortunately I hit an unexpected error while processing your comment. I've automatically reported this to GitHub. You can ask me to try again later by mentioning me in a new comment. If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: Sorry for the inconvenience! |
…sues-from-codex-review-#154 # Conflicts: # src/parity_audit.py Co-authored-by: badMade <106821302+badMade@users.noreply.github.com>
Resolved the merge conflict in |
Motivation
run_parity_audit()from erroneously treating the presence oftask.pyas coverage for the archivedtasksdirectory, which masked missing parity for the tasks subsystem.Description
ARCHIVE_DIR_MAPPINGS['tasks']from'task.py'to'tasks'insrc/parity_audit.pyso directory parity is tracked independently and commit the change.Testing
python3 -m src.parity_auditand a small snippet (from src.parity_audit import run_parity_audit) which showeddirectory_coverageat34/35and thattasksis listed inmissing_directory_targets, and noted thatPYTHONPATH=. python3 src/parity_audit.pystill fails due to an unrelatedsrc/typesimport-shadowing issue.Codex Task