Skip to content

Commit e8eff18

Browse files
committed
Merge upstream/main into feat/trace-line-ranges
Resolve conflicts with the rename support of #3659 and the Pi column of the Trace README. Clarify in the README when a range has no attribution. Assisted-by: Claude Code Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
2 parents 2edc07f + 6767675 commit e8eff18

52 files changed

Lines changed: 3285 additions & 314 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎app/cli/internal/trace/README.md‎

Lines changed: 54 additions & 50 deletions
Large diffs are not rendered by default.

‎app/cli/internal/trace/attribution/enrich.go‎

Lines changed: 102 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
package attribution
1717

1818
import (
19+
"maps"
20+
"slices"
21+
1922
"github.com/chainloop-dev/chainloop/app/cli/internal/trace"
2023
tracegit "github.com/chainloop-dev/chainloop/app/cli/internal/trace/git"
2124
"github.com/chainloop-dev/chainloop/app/cli/internal/trace/state"
@@ -63,6 +66,12 @@ type CommitLines struct {
6366
// (R-003). A file that tip does not hold keeps the AI ranges at edit time,
6467
// with no attribution, as older CLIs write them (R-005).
6568
//
69+
// A file that was renamed after a commit of the session has the lines of its
70+
// new path in tip (R-007). A line there counts for the path that its commit
71+
// changed, so a session that changed the file under both paths counts each
72+
// line once. A commit that renamed a file matches the ledger of the new path,
73+
// then of the old one.
74+
//
6675
// A file with an edit recorded before line fingerprints existed keeps the
6776
// earlier rule: AI lines are the recorded ranges, capped at the file's diff.
6877
func Enrich(sessionID string, ledgers map[string]*state.AILineAttribution, commits []CommitLines, tip map[string][]tracegit.BlameLine, changes *aicodingsession.CodeChanges) {
@@ -83,15 +92,25 @@ func Enrich(sessionID string, ledgers map[string]*state.AILineAttribution, commi
8392
positioned[path] = net.ranges
8493
}
8594

95+
oldPaths := make(map[string][]string)
96+
for _, c := range commits {
97+
for _, fl := range c.Files {
98+
if fl.OldPath != "" && !slices.Contains(oldPaths[fl.Path], fl.OldPath) {
99+
oldPaths[fl.Path] = append(oldPaths[fl.Path], fl.OldPath)
100+
}
101+
}
102+
}
103+
86104
changes.AILinesAdded, changes.AILinesRemoved = 0, 0
87105
changes.HumanLinesAdded, changes.HumanLinesRemoved = 0, 0
88106
for i := range changes.Files {
89107
f := &changes.Files[i]
90108

91109
// The marker keeps its earlier meaning, a file that the session
92-
// edited, for platforms that do not read the split (R-009). An empty
93-
// ranges slice is valid: the agent only deleted lines.
94-
if aiRanges, ok := self.Files[f.Path]; ok {
110+
// edited, also under the path that a commit renamed, for platforms
111+
// that do not read the split (R-009). An empty ranges slice is valid:
112+
// the agent only deleted lines.
113+
if aiRanges, ok := editedRanges(self, f.Path, oldPaths[f.Path]); ok {
95114
f.Attribution = trace.AttributionAI
96115
f.LineRanges = aiRanges
97116
f.SessionIDs = []string{sessionID}
@@ -141,6 +160,18 @@ func (s *fileSplit) addLine(n int, owner string) {
141160
s.ranges = append(s.ranges, aicodingsession.LineRange{Start: n, End: n, Attribution: owner})
142161
}
143162

163+
// editedRanges returns the recorded ranges of the session's edits of a file,
164+
// under its path or, when a commit renamed it, under an old path.
165+
func editedRanges(l *state.AILineAttribution, path string, oldPaths []string) ([]aicodingsession.LineRange, bool) {
166+
for _, p := range append([]string{path}, oldPaths...) {
167+
if ranges, ok := l.Files[p]; ok {
168+
return ranges, true
169+
}
170+
}
171+
172+
return nil, false
173+
}
174+
144175
// splitByPath matches the lines of each commit against the ledgers, and
145176
// returns the split of each file that the commits changed.
146177
func splitByPath(sessionID string, self *state.AILineAttribution, ledgers map[string]*state.AILineAttribution, commits []CommitLines) map[string]*fileSplit {
@@ -166,19 +197,19 @@ func splitByPath(sessionID string, self *state.AILineAttribution, ledgers map[st
166197
for _, line := range fl.Added {
167198
fp := Fingerprint(line)
168199
switch {
169-
case written.take(self, fl.Path, fp):
200+
case written.takeFile(self, fl, fp):
170201
s.aiAdded++
171-
case written.takeAny(others, fl.Path, fp):
202+
case written.takeAny(others, fl, fp):
172203
default:
173204
s.humanAdded++
174205
}
175206
}
176207
for _, line := range fl.Removed {
177208
fp := Fingerprint(line)
178209
switch {
179-
case erased.take(self, fl.Path, fp):
210+
case erased.takeFile(self, fl, fp):
180211
s.aiRemoved++
181-
case erased.takeAny(others, fl.Path, fp):
212+
case erased.takeAny(others, fl, fp):
182213
default:
183214
s.humanRemoved++
184215
}
@@ -202,39 +233,41 @@ func netAdded(sessionID string, self *state.AILineAttribution, ledgers map[strin
202233
written := linePool{pick: func(a *state.AILineAttribution) map[string]state.LineCounts { return a.Written }}
203234
joined := make(map[[2]string]state.LineCounts)
204235

236+
// Two paths of the session can have the same lines in tip, after a
237+
// rename. The order makes the split of such lines the same on each push.
205238
out := make(map[string]*fileSplit, len(tip))
206-
for path, lines := range tip {
239+
for _, path := range slices.Sorted(maps.Keys(tip)) {
207240
s := &fileSplit{}
208241
out[path] = s
209242

210-
for i, l := range lines {
243+
for i, l := range tip[path] {
211244
commit, ok := bySHA[l.SHA]
212245
if !ok {
213246
continue
214247
}
215248
fp := Fingerprint(l.Text)
216249

217250
if commit.Joined != nil {
218-
key := [2]string{commit.SHA, path}
219-
pool, ok := joined[key]
220-
if !ok {
221-
pool = state.LineCounts{}
222-
pool.Add(commit.Joined[path]...)
223-
joined[key] = pool
224-
}
225-
if pool.Take(fp) {
251+
if joinedPool(joined, commit, path, tip).Take(fp) {
226252
s.humanAdded++
227253
s.addLine(i+1, trace.AttributionHuman)
228254
}
229255

230256
continue
231257
}
232258

259+
// A line that the commit wrote under another path of the
260+
// session counts for that path.
261+
fl, ok := commit.file(path)
262+
if !ok {
263+
continue
264+
}
265+
233266
switch {
234-
case written.take(self, path, fp):
267+
case written.takeFile(self, fl, fp):
235268
s.aiAdded++
236269
s.addLine(i+1, trace.AttributionAI)
237-
case written.takeAny(otherLedgers(sessionID, ledgers, commit.SessionIDs), path, fp):
270+
case written.takeAny(otherLedgers(sessionID, ledgers, commit.SessionIDs), fl, fp):
238271
default:
239272
s.humanAdded++
240273
s.addLine(i+1, trace.AttributionHuman)
@@ -245,6 +278,47 @@ func netAdded(sessionID string, self *state.AILineAttribution, ledgers map[strin
245278
return out
246279
}
247280

281+
// file returns the lines that the commit changed in path.
282+
func (c CommitLines) file(path string) (tracegit.FileLines, bool) {
283+
for _, fl := range c.Files {
284+
if fl.Path == path {
285+
return fl, true
286+
}
287+
}
288+
289+
return tracegit.FileLines{}, false
290+
}
291+
292+
// joinedPool returns the human lines that a joined commit added to path, one
293+
// copy per match. A joined commit that changed the file under a path that tip
294+
// does not hold, because a later commit renamed it, shares one pool of those
295+
// lines across the paths of tip.
296+
func joinedPool(pools map[[2]string]state.LineCounts, commit CommitLines, path string, tip map[string][]tracegit.BlameLine) state.LineCounts {
297+
key := [2]string{commit.SHA, path}
298+
if _, direct := commit.Joined[path]; !direct {
299+
key[1] = ""
300+
}
301+
302+
pool, ok := pools[key]
303+
if ok {
304+
return pool
305+
}
306+
307+
pool = state.LineCounts{}
308+
if key[1] != "" {
309+
pool.Add(commit.Joined[path]...)
310+
} else {
311+
for p, fps := range commit.Joined {
312+
if _, inTip := tip[p]; !inTip {
313+
pool.Add(fps...)
314+
}
315+
}
316+
}
317+
pools[key] = pool
318+
319+
return pool
320+
}
321+
248322
// otherLedgers returns the ledgers of the sessions in ids other than
249323
// sessionID.
250324
func otherLedgers(sessionID string, ledgers map[string]*state.AILineAttribution, ids []string) []*state.AILineAttribution {
@@ -283,10 +357,16 @@ func (p *linePool) take(l *state.AILineAttribution, path, fp string) bool {
283357
return c.Take(fp)
284358
}
285359

286-
// takeAny takes fp from the first ledger that holds it.
287-
func (p *linePool) takeAny(ledgers []*state.AILineAttribution, path, fp string) bool {
360+
// takeFile takes fp from the lines of ledger l for the file, under its path,
361+
// and then under its old path when the commit renamed it.
362+
func (p *linePool) takeFile(l *state.AILineAttribution, fl tracegit.FileLines, fp string) bool {
363+
return p.take(l, fl.Path, fp) || (fl.OldPath != "" && p.take(l, fl.OldPath, fp))
364+
}
365+
366+
// takeAny takes fp from the first ledger that holds it for the file.
367+
func (p *linePool) takeAny(ledgers []*state.AILineAttribution, fl tracegit.FileLines, fp string) bool {
288368
for _, l := range ledgers {
289-
if p.take(l, path, fp) {
369+
if p.takeFile(l, fl, fp) {
290370
return true
291371
}
292372
}

‎app/cli/internal/trace/attribution/enrich_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ func ledger(sessionID, file string, written, erased []string) *state.AILineAttri
5858
}
5959
}
6060

61+
// oldFile is the path of testFile before a rename.
62+
const oldFile = "old.go"
63+
64+
// counts holds the fingerprints of lines.
65+
func counts(lines ...string) state.LineCounts {
66+
c := state.LineCounts{}
67+
c.Add(fps(lines...)...)
68+
69+
return c
70+
}
71+
6172
func TestEnrich_Legacy(t *testing.T) {
6273
t.Run("no AI data attributes all changes to human", func(t *testing.T) {
6374
changes := &aicodingsession.CodeChanges{
@@ -255,6 +266,16 @@ func TestEnrich_Lines(t *testing.T) {
255266
},
256267
want: split{aiAdded: 2, humanAdded: 1, humanRemoved: 1},
257268
},
269+
{
270+
// R-007: the agent edited the file under its old name, and the
271+
// commit renamed it.
272+
name: "a renamed file matches the ledger of its old path",
273+
ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, oldFile, []string{"a"}, []string{"x"})},
274+
commits: []CommitLines{{SessionIDs: []string{testSession}, Files: []tracegit.FileLines{
275+
{Path: testFile, OldPath: oldFile, Added: []string{"a", "b"}, Removed: []string{"x", "y"}},
276+
}}},
277+
want: split{aiAdded: 1, aiRemoved: 1, humanAdded: 1, humanRemoved: 1},
278+
},
258279
}
259280

260281
for _, tc := range testCases {
@@ -378,6 +399,44 @@ func TestEnrich_Net(t *testing.T) {
378399
want: split{humanAdded: 2},
379400
ranges: []aicodingsession.LineRange{human(2, 3)},
380401
},
402+
{
403+
// R-007: the session wrote old.go, a commit renamed it to the
404+
// file, and a later commit of the session changed the file. The
405+
// tip of both paths is the file at HEAD, and each line counts for
406+
// the path that its commit changed.
407+
name: "a line counts for the path that its commit changed",
408+
ledgers: map[string]*state.AILineAttribution{testSession: {
409+
SessionID: testSession,
410+
Written: map[string]state.LineCounts{
411+
oldFile: counts(five...),
412+
testFile: counts("f"),
413+
},
414+
}},
415+
commits: []CommitLines{
416+
{SHA: c1, SessionIDs: []string{testSession}, Files: []tracegit.FileLines{{Path: oldFile, Added: five}}},
417+
ownCommit(c2, []string{"f"}, nil),
418+
},
419+
tip: map[string][]tracegit.BlameLine{
420+
oldFile: tipOf(c1+":a", c1+":b", c1+":c", c1+":d", c1+":e", c2+":f"),
421+
testFile: tipOf(c1+":a", c1+":b", c1+":c", c1+":d", c1+":e", c2+":f"),
422+
},
423+
want: split{aiAdded: 1},
424+
ranges: []aicodingsession.LineRange{ai(6, 6)},
425+
},
426+
{
427+
// R-007: a commit in no session renamed the file and replaced a
428+
// line of the session in it.
429+
name: "a joined commit counts in a renamed file",
430+
ledgers: map[string]*state.AILineAttribution{testSession: ledger(testSession, testFile, five, nil)},
431+
commits: []CommitLines{
432+
ownCommit(c1, five, nil),
433+
{SHA: c2, Files: []tracegit.FileLines{{Path: "new.go", OldPath: testFile, Added: []string{"C"}, Removed: []string{"c"}}},
434+
Joined: map[string][]string{"new.go": fps("C")}},
435+
},
436+
tip: map[string][]tracegit.BlameLine{testFile: tipOf(c1+":a", c1+":b", c2+":C", c1+":d", c1+":e")},
437+
want: split{aiAdded: 4, humanAdded: 1},
438+
ranges: []aicodingsession.LineRange{ai(1, 2), human(3, 3), ai(4, 5)},
439+
},
381440
}
382441

383442
for _, tc := range testCases {

‎app/cli/internal/trace/e2e/checks.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"io/fs"
2323
"os"
24+
"path"
2425
"path/filepath"
2526
"strings"
2627

@@ -166,7 +167,11 @@ func checkModel(s *Session) error {
166167
if m == nil || m.Primary == "" {
167168
return errors.New("the evidence has no primary model")
168169
}
169-
if !strings.Contains(strings.ToLower(m.Primary), strings.ToLower(s.Model)) {
170+
// A model can have a provider prefix, for example
171+
// amazon-bedrock/eu.anthropic.claude-haiku-5-5, that the evidence leaves
172+
// out, and a short name, for example haiku, that the evidence spells out.
173+
primary, want := strings.ToLower(m.Primary), strings.ToLower(path.Base(s.Model))
174+
if !strings.Contains(primary, want) && !strings.Contains(want, primary) {
170175
return fmt.Errorf("primary model %q is not the model of the session %q", m.Primary, s.Model)
171176
}
172177
return nil
@@ -357,15 +362,23 @@ func checkSecretRedacted(s *Session) error {
357362
// secret, as internal/redaction writes it.
358363
const redactedPlaceholder = "[CHAINLOOP_TRACE_REDACTED"
359364

365+
// checkAllowedSecretKept checks that the secret on the line with an allow
366+
// marker is in the stored session. The line comes from the output of the
367+
// tool that read the fixture. A transcript with no tool outputs holds no
368+
// copy of the line, so it cannot show the feature.
360369
func checkAllowedSecretKept(s *Session) error {
361370
found, err := storedContains(s, fakeAllowedSecret)
371+
if err != nil || found {
372+
return err
373+
}
374+
marked, err := storedContains(s, allowMarker)
362375
if err != nil {
363376
return err
364377
}
365-
if !found {
366-
return errors.New("the secret on the line with an allow marker was redacted, or the session did not read it")
378+
if !marked {
379+
return fmt.Errorf("%w: the stored session holds no copy of the fixture line, for example because the transcript has no tool outputs", errNotCovered)
367380
}
368-
return nil
381+
return errors.New("the secret on the line with an allow marker was redacted")
369382
}
370383

371384
func checkCommitTrailer(s *Session) error {

0 commit comments

Comments
 (0)