Update /project commands from upstream tnf-dev-env#101
Update /project commands from upstream tnf-dev-env#101fonta-rh wants to merge 3 commits intoopenshift-eng:mainfrom
Conversation
Sync the /project:new, /project:resume, and /project:close commands with the latest versions from fonta-rh/tnf-dev-env. Key changes: - Extract project resolution logic into shared resume-project.py script - Introduce lean index pattern for project CLAUDE.md (~50-80 lines) - Add detail file templates (investigation.md, ci-runs.md, etc.) - Implement lazy repo context loading in /project:resume - Add Reference Files table as manifest for detail file discovery Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fonta-rh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughReplaces interactive project selection and monolithic CLAUDE.md usage with a new CLI tool ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
multi-repo-development/scripts/resume-project.py (1)
196-202: Non-deterministic preset selection when multiple matches exist.
glob.globreturns matches in arbitrary filesystem order. If multiple presets contain context for the same repo (e.g.,presets/a/context/repo.mdandpresets/b/context/repo.md), the selected file is non-deterministic.Consider sorting or documenting the expected behavior:
Option: Sort matches for deterministic selection
matches = glob.glob(str(root / "presets" / "*" / "context" / f"{repo}.md")) if matches: + matches.sort() # Deterministic: pick alphabetically first preset results.append({ "repo": repo, "path": str(Path(matches[0]).relative_to(root)),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@multi-repo-development/scripts/resume-project.py` around lines 196 - 202, The current selection of a preset file using matches = glob.glob(...) is non-deterministic when multiple matching files exist; change the logic around matches to enforce deterministic selection by sorting the matches list (e.g., sorted(matches)) and then using the first element, or otherwise define and apply a clear deterministic rule (e.g., choose the lexicographically smallest path) before calling results.append with "repo", "path" and "source"; update any comments to document the chosen deterministic behavior so future readers know why the first match is selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@multi-repo-development/scripts/resume-project.py`:
- Line 220: Rename the ambiguous list-comprehension variable `l` to a clearer
name (e.g., `line`) to satisfy ruff E741 and PEP 8; update the return statement
in the function containing the list comprehension (the one that returns
processed stdout lines) to use the new variable name `line` everywhere in that
expression so it reads like: return [line.strip() for line in
result.stdout.splitlines() if line.strip()].
- Around line 165-167: The checklist regex only accepts lowercase "x" so
uppercase checked items are treated as unchecked; update the regex used where
item_match is computed (the re.match call that assigns item_match and then sets
done) to accept both "x" and "X" (e.g., change the pattern to r"^\s*- \[([
xX])\] (.+)$" or add the re.IGNORECASE flag) so done = item_match.group(1) ==
"x" still works (or normalize with .lower() when comparing).
- Around line 114-118: The separator regex in the block that checks found_header
and not skipped_separator uses re.match(r"^\|[-\s|]+\|$", line) and will fail on
lines with trailing whitespace (e.g., "|---| "), causing data rows to be
skipped; fix by trimming trailing whitespace before matching (e.g., use
line.rstrip() when calling re.match) or broaden the regex to allow optional
trailing spaces (e.g., r"^\|[-\s|]+\|\s*$") in the same conditional inside the
loop that uses found_header and skipped_separator so skipped_separator is set
correctly.
- Around line 1-6: Add positive and negative unit tests covering the regex-based
parsers parse_frontmatter, parse_reference_files, and extract_checklist: include
cases for valid YAML frontmatter, missing/incorrect delimiters, mixed scalars vs
lists, and empty values; tests for reference files table with trailing
whitespace, missing columns, malformed/Markdown-formatted cells, and header/row
boundary conditions; and checklist variations including "[x]" vs "[ ]", nested
checklist items, items in different sections, case/whitespace variations, and
boundary conditions for the regexes. Ensure each test asserts expected parsed
output or a specific failure/empty result where malformed input is used, and add
both positive (should parse) and negative (should fail/return empty) tests for
each parser to satisfy CONTRIBUTING.md.
---
Nitpick comments:
In `@multi-repo-development/scripts/resume-project.py`:
- Around line 196-202: The current selection of a preset file using matches =
glob.glob(...) is non-deterministic when multiple matching files exist; change
the logic around matches to enforce deterministic selection by sorting the
matches list (e.g., sorted(matches)) and then using the first element, or
otherwise define and apply a clear deterministic rule (e.g., choose the
lexicographically smallest path) before calling results.append with "repo",
"path" and "source"; update any comments to document the chosen deterministic
behavior so future readers know why the first match is selected.
🪄 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: CHILL
Plan: Enterprise
Run ID: 32345780-afd2-4f62-b651-ae4709ae65af
📒 Files selected for processing (4)
multi-repo-development/.claude/commands/project/close.mdmulti-repo-development/.claude/commands/project/new.mdmulti-repo-development/.claude/commands/project/resume.mdmulti-repo-development/scripts/resume-project.py
- Tolerate trailing whitespace in markdown table separator regex - Accept uppercase [X] in checklist parsing - Rename ambiguous variable `l` to `line` (ruff E741) Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
multi-repo-development/scripts/resume-project.py (1)
47-93:⚠️ Potential issue | 🟠 MajorAdd tests for the new parsing and status-resolution paths.
Per CONTRIBUTING.md, this PR adds regex/parsing/validation logic (
parse_frontmatter,parse_reference_files,extract_checklist, and status branching inresolve_project) but does not include positive/negative tests in this change set. Please add coverage for valid and malformed inputs, plus status outcomes (ok,no_argument,not_found,out_of_range,no_projects).As per coding guidelines, "regex/parsing/validation logic (e.g., CLI JSON status branching, reference-file parsing, checklist extraction) should include positive/negative tests."
Also applies to: 96-130, 153-180, 233-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@multi-repo-development/scripts/resume-project.py` around lines 47 - 93, Add unit tests covering positive and negative cases for the new parsing/validation logic: create tests for parse_frontmatter (valid YAML frontmatter with scalars and lists, empty/malformed frontmatter, and missing file), parse_reference_files (valid references, malformed refs, missing files), and extract_checklist (well-formed checklist items and broken/misaligned lists). Also add tests for resolve_project asserting all status branches: "ok", "no_argument", "not_found", "out_of_range", and "no_projects" by stubbing inputs/responses as needed; reference the functions parse_frontmatter, parse_reference_files, extract_checklist, and resolve_project when locating where to add tests. Ensure both positive (expected success outputs) and negative (errors/edge cases) assertions are present for each path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@multi-repo-development/scripts/resume-project.py`:
- Around line 213-217: Add a timeout to the subprocess.run calls that invoke
external scripts to avoid hangs: modify the subprocess.run invocation that
executes recent-projects.py (the call assigning to variable result which uses
[sys.executable, str(script), "--names"] and env={**os.environ,
"CLAUDE_PROJECT_DIR": str(root)}) to include a timeout parameter (e.g.,
timeout=30); apply the same change to the similar subprocess.run call around the
later block (the one at lines 347-350) so both invocations are bounded.
---
Duplicate comments:
In `@multi-repo-development/scripts/resume-project.py`:
- Around line 47-93: Add unit tests covering positive and negative cases for the
new parsing/validation logic: create tests for parse_frontmatter (valid YAML
frontmatter with scalars and lists, empty/malformed frontmatter, and missing
file), parse_reference_files (valid references, malformed refs, missing files),
and extract_checklist (well-formed checklist items and broken/misaligned lists).
Also add tests for resolve_project asserting all status branches: "ok",
"no_argument", "not_found", "out_of_range", and "no_projects" by stubbing
inputs/responses as needed; reference the functions parse_frontmatter,
parse_reference_files, extract_checklist, and resolve_project when locating
where to add tests. Ensure both positive (expected success outputs) and negative
(errors/edge cases) assertions are present for each path.
🪄 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: CHILL
Plan: Enterprise
Run ID: a26892d8-eb47-4a1f-afd5-038eeffb3ba9
📒 Files selected for processing (1)
multi-repo-development/scripts/resume-project.py
|
/test markdownlint |
|
@fonta-rh: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Convert bold step labels to ### subheadings (MD036), add blank lines around lists (MD032), add language specifiers to fenced code blocks (MD040), indent table under ordered list item to preserve numbering (MD029), and escape angle brackets in inline examples (MD033). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
/project:new,/project:resume, and/project:closecommands with the latest versions from fonta-rh/tnf-dev-envscripts/resume-project.py— shared project resolution logic used by both/project:resumeand/project:close, replacing duplicated inline resolution in each command🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor