Skip to content

Address quality issues from PR #13 review#20

Merged
fazxes merged 1 commit intomainfrom
claude/review-pr13-fpEy7
Apr 4, 2026
Merged

Address quality issues from PR #13 review#20
fazxes merged 1 commit intomainfrom
claude/review-pr13-fpEy7

Conversation

@fazxes
Copy link
Copy Markdown
Member

@fazxes fazxes commented Apr 4, 2026

Summary

Quality improvements identified during code review of PR #13 ("Harden Nightshift from live validation runs").

Test plan

  • Existing tests for high_signal_focus_paths still pass with simplified logic
  • New tests for bash -c and sh -c shell wrapper detection pass
  • make check passes (ruff, mypy, pytest)
  • __all__ sort order in __init__.py may need updating for ruff RUF022

https://claude.ai/code/session_011hjFhGSxRM4pWsWN1r4GSM

Copy link
Copy Markdown
Member Author

fazxes commented Apr 4, 2026

Note: make check will likely fail on ruff RUF022 (__all__ is not sorted) in nightshift/__init__.py. This predates the changes in this PR but will need fixing to pass CI. The __all__ list needs isort-style alphabetical sorting.


Generated by Claude Code

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a66d23e5e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

Comment thread nightshift/constants.py
# Python
"app/models",
"app/auth",
"app/api",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Deduplicate high-signal path candidates

The new entries add duplicates of existing candidates (for example app/api is already present earlier in the same list), which causes high_signal_focus_paths() to emit repeated hints instead of distinct areas to explore. In repos where only a duplicated path exists, the focus list can become ['app/api', 'app/api'] (and similarly for src/api), reducing prompt quality and path diversity for each cycle.

Useful? React with 👍 / 👎.

- Simplify high_signal_focus_paths() by removing redundant fallback (#16)
- Broaden _extract_shell_command regex to handle bash/sh/dash wrappers (#17)
- Add docstring to expected_cycle_commits() documenting commit contract (#18)
- Add Python/Go/Rust paths to HIGH_SIGNAL_PATH_CANDIDATES (#19)

https://claude.ai/code/session_011hjFhGSxRM4pWsWN1r4GSM
@fazxes fazxes force-pushed the claude/review-pr13-fpEy7 branch from a66d23e to 58512da Compare April 4, 2026 01:50
@fazxes fazxes merged commit 950438c into main Apr 4, 2026
2 checks passed
@fazxes fazxes deleted the claude/review-pr13-fpEy7 branch April 4, 2026 01:50
fazxes added a commit that referenced this pull request Apr 5, 2026
nightshift plan "feature" --agent claude now invokes the agent to generate
a feature plan, parses the output, and displays formatted markdown with
scope warnings. Adds plan_command_for_agent() and run_plan_agent() to
planner.py with PLAN_AGENT_MAX_TURNS (10) and PLAN_AGENT_TIMEOUT (300s).
13 new tests, 509 total passing. Completes task #20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants