Fix immediate stats after editor agent commits#1640
Conversation
Co-authored-by: Codex <noreply@openai.com>
08eac5f to
f4028a2
Compare
f4028a2 to
79aa8a1
Compare
|
I would strongly prefer us not using daemon syncs in production -- it's really only meant for testing. Could we just check if it's a recent commit and it doesn't have a note, wait a couple seconds and try again? Or even just add special log output for these cases for the user to try again later? The other thing to keep in mind, is it's easily possible for the daemon to be 30+ seconds behind if its processing a big rebase or something like that for the repo |
|
@svarlamov - I remember we agreed on the above approach! What would you suggest as an alternative? |
| fn sync_daemon_before_status_read(repo: &Repository) { | ||
| let Ok(workdir) = repo.workdir() else { | ||
| return; | ||
| }; | ||
| let Ok(config) = DaemonConfig::from_env_or_default_paths() else { | ||
| return; | ||
| }; | ||
| let request = ControlRequest::SyncFamily { | ||
| repo_working_dir: workdir.to_string_lossy().to_string(), | ||
| }; | ||
| match send_control_request(&config.control_socket_path, &request) { | ||
| Ok(response) if response.ok => {} | ||
| Ok(response) => { | ||
| tracing::debug!( | ||
| "daemon sync before status failed: {}", | ||
| response | ||
| .error | ||
| .unwrap_or_else(|| "unknown error".to_string()) | ||
| ); | ||
| } | ||
| Err(error) => { | ||
| tracing::debug!("daemon sync before status unavailable: {}", error); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Identical daemon-sync helper is duplicated across two files instead of being shared
The daemon sync logic is copy-pasted verbatim into two separate private functions (sync_daemon_before_stats_read at src/commands/git_ai_handlers.rs:1062 and sync_daemon_before_status_read at src/commands/status.rs:218) differing only in a debug log string, rather than being extracted into a single reusable helper.
Impact: Violates the repository's mandatory code-reuse rule and increases maintenance burden for any future changes to the sync logic.
AGENTS.md rule #5 violation details
AGENTS.md states: "Reuse existing code. This is a large codebase. For nearly any operation you need, there is almost certainly a helper function already available and tested. Reuse code as much as practical -- it also keeps diffs small. PRs that fail to reuse existing code where applicable will be rejected immediately."
Both functions have identical structure:
- Get workdir from repo
- Get DaemonConfig from env/default paths
- Build a
ControlRequest::SyncFamilywith the workdir - Call
send_control_requestand handle Ok/Err
The only difference is the debug log message ("stats" vs "status"). These should be a single shared function, possibly accepting a &str context label for the debug message.
src/commands/git_ai_handlers.rs:1062-1086 is identical to src/commands/status.rs:218-242.
Prompt for agents
The functions sync_daemon_before_stats_read (in src/commands/git_ai_handlers.rs:1062-1086) and sync_daemon_before_status_read (in src/commands/status.rs:218-242) are identical except for the debug log message string. Extract a single shared helper function (e.g., in a common module or as a public utility) that accepts a context label parameter (like "stats" or "status") for the debug messages. Then have both call sites use that shared helper. This satisfies the AGENTS.md rule about reusing existing code and keeping diffs small.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
CIs will be better after #1734 |
Summary
git-ai statsandgit-ai statusread attribution dataFixes #1423.
Refs #1584.
Validation
Run from the repo root on this branch:
Expected outcomes:
task fmtexits cleanly with no rustfmt diffs.task lintexits cleanly with no clippy/lint failures.git-ai status --jsonand immediategit-ai stats HEAD --jsonreportai_additions == 1after the delayed daemon commit path.ai_additions == 1expectation.ai_additions == 0and keeps the new lines as unknown/untracked additions.Broader regression sweep:
Expected outcome: stats, Cursor, and GitHub Copilot integration coverage pass with the daemon sync behavior enabled.
Additional whitespace check:
Expected outcome: no output and exit code
0.Docs
No docs change needed. This fixes timing of existing
git-ai stats/git-ai statusbehavior and is covered by regression tests.Notes
Draft PR for focused review and CI validation.