-
Notifications
You must be signed in to change notification settings - Fork 0
feat: include commit message bodies in memory context (GIT-104) #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -272,4 +272,72 @@ export class GitClient implements IGitClient { | |||||||||||||
| return []; | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| getCommitMessages(shas: readonly string[], cwd?: string): Map<string, { subject: string; body: string }> { | ||||||||||||||
| const result = new Map<string, { subject: string; body: string }>(); | ||||||||||||||
|
|
||||||||||||||
| if (shas.length === 0) return result; | ||||||||||||||
|
|
||||||||||||||
| // Deduplicate SHAs | ||||||||||||||
| const uniqueShas = [...new Set(shas)]; | ||||||||||||||
|
|
||||||||||||||
| // Use git log with specific SHAs to fetch all at once | ||||||||||||||
| // Format: SHA<FS>subject<FS>body<RS> | ||||||||||||||
| const format = ['%H', '%s', '%b'].join(GitClient.FIELD_SEP); | ||||||||||||||
|
|
||||||||||||||
| try { | ||||||||||||||
| // We use --no-walk to avoid following parents, just show the specified commits | ||||||||||||||
| const output = execFileSync( | ||||||||||||||
| 'git', | ||||||||||||||
| [ | ||||||||||||||
| 'log', | ||||||||||||||
| '--no-walk', | ||||||||||||||
| `--format=${GitClient.RECORD_SEP}${format}`, | ||||||||||||||
| ...uniqueShas, | ||||||||||||||
| ], | ||||||||||||||
| { | ||||||||||||||
| encoding: 'utf8', | ||||||||||||||
| cwd, | ||||||||||||||
| stdio: ['pipe', 'pipe', 'pipe'], | ||||||||||||||
| maxBuffer: 10 * 1024 * 1024, | ||||||||||||||
| } | ||||||||||||||
| ).trim(); | ||||||||||||||
|
|
||||||||||||||
| if (!output) return result; | ||||||||||||||
|
|
||||||||||||||
| const records = output.split(GitClient.RECORD_SEP).filter(r => r.trim()); | ||||||||||||||
| for (const record of records) { | ||||||||||||||
| const fields = record.split(GitClient.FIELD_SEP); | ||||||||||||||
| const sha = fields[0] || ''; | ||||||||||||||
| const subject = fields[1] || ''; | ||||||||||||||
| const body = (fields[2] || '').trim(); | ||||||||||||||
|
|
||||||||||||||
| if (sha) { | ||||||||||||||
| result.set(sha, { subject, body }); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } catch { | ||||||||||||||
| // If batch fails, try individual lookups as fallback | ||||||||||||||
| for (const sha of uniqueShas) { | ||||||||||||||
| try { | ||||||||||||||
| const output = execFileSync( | ||||||||||||||
| 'git', | ||||||||||||||
| ['log', '-1', `--format=%s${GitClient.FIELD_SEP}%b`, sha], | ||||||||||||||
| { | ||||||||||||||
| encoding: 'utf8', | ||||||||||||||
| cwd, | ||||||||||||||
| stdio: ['pipe', 'pipe', 'pipe'], | ||||||||||||||
| } | ||||||||||||||
| ).trim(); | ||||||||||||||
|
|
||||||||||||||
| const [subject, body] = output.split(GitClient.FIELD_SEP); | ||||||||||||||
| result.set(sha, { subject: subject || '', body: (body || '').trim() }); | ||||||||||||||
|
Comment on lines
+333
to
+334
|
||||||||||||||
| const [subject, body] = output.split(GitClient.FIELD_SEP); | |
| result.set(sha, { subject: subject || '', body: (body || '').trim() }); | |
| const fields = output.split(GitClient.FIELD_SEP); | |
| const subject = fields[0] || ''; | |
| const body = fields.slice(1).join(GitClient.FIELD_SEP).trim(); | |
| result.set(sha, { subject, body }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Solid implementation with batch optimization and fallback.
The approach is sound: batch query with --no-walk for efficiency, deduplication via Set, and per-SHA fallback for resilience.
Minor edge case: If a commit body contains the field separator \x1f, the body would be truncated at line 333. This is extremely rare in practice (ASCII Unit Separator in commit messages), but for robustness you could use a limit parameter on split:
🔧 Optional fix for body containing field separator
- const [subject, body] = output.split(GitClient.FIELD_SEP);
+ const parts = output.split(GitClient.FIELD_SEP);
+ const subject = parts[0] || '';
+ const body = parts.slice(1).join(GitClient.FIELD_SEP).trim();The same pattern could be applied to the batch parsing at lines 310-313 if desired.
🤖 Prompt for AI Agents
In `@src/infrastructure/git/GitClient.ts` around lines 276 - 342, The parsing in
getCommitMessages can truncate commit bodies if they contain the FIELD_SEP
(GitClient.FIELD_SEP); change the splits to limit to three fields so only the
first two separators are honored (e.g., use split with a limit of 3) when
parsing both the batch records (records.map/for loop that assigns sha, subject,
body) and the per-commit fallback (where output is split into subject and body)
so the body retains any embedded separators; update references around
GitClient.RECORD_SEP, GitClient.FIELD_SEP, and the getCommitMessages parsing
logic accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use a type guard for proper type narrowing.
filter(Boolean)removes falsy values at runtime but TypeScript doesn't narrow the resulting type. Ifshacan beundefined, the array type remains(string | undefined)[]rather thanstring[].🛡️ Proposed fix for type safety
📝 Committable suggestion
🤖 Prompt for AI Agents