Add numeric scoring with weights configurable via cli

[omkar-foss]

Closes #12.

This PR adds numeric scoring based on weights which are configurable via cli, with supporting tests (which is most of the diff here).

Additionally:

1. this fixes a panic when git hash passed is less than 12 characters. Although this may or may not possibly be a real occurrence, I think we should validate the slicing and avoid panics.
2. moves output.ConfidenceFromString(minConfFlag) to detection.ConfidenceFromString(minConfFlag) to keep all Confidence-related functionality together.

[omkar-foss]

Just for reference, implementation of numeric scoring here is based on this: #12 (comment)

[andrew]

Thanks for taking this on, and for the thorough tests.

The main thing I'd like to revisit is the consolidation step. ConsolidateFindingScore takes the max score per detector and then averages across detectors, so a commit with just a Co-Authored-By trailer scores 85, but the same commit with an additional tool mention in the message body drops to 52.5. Adding corroborating evidence shouldn't lower the score. #12 was heading toward something additive (SpamAssassin-style) rather than an average — probably worth pinning that down on the issue before reworking the code.

Related: the repo-wide OverallScore pools every finding from every commit into one max-per-detector average, so for a 500-commit range with one AI commit it just reports that one commit's score. I'm not sure a single number across a range is meaningful; the per-commit score is the useful bit.

A couple of structural things:

• confidenceScores and scan.Weights are package-level mutable state set from the CLI. --confidence-scores changes what every detector reports as Confidence and never resets, which leaks across Run() calls (and between tests — TestRunScanScoreFlags leaves it modified). Would prefer these threaded through as arguments rather than globals.
• SetConfidenceScoresFromStrings doesn't check the thresholds are ordered, so low=50,medium=30 makes ScoreToConfidence(40) return low.

Minor:

• strconv.ParseFloat(fmt.Sprintf("%.2f", overall), 64) → math.Round(overall*100)/100
• Replit Agent and Assistant used to be medium vs low confidence; both are now TrailerMatchBaseScore — intentional?
• The IIFE in FormatJSONFindings can be a plain local.
• Stray blank line at committer.go:28, typo overridence in detection.go.

The hash-slicing panic fix and the ConfidenceFromString move are both good and would happily take those as a separate PR if you want them in sooner.

[omkar-foss]

Thanks for your review, my comments below.

The main thing I'd like to revisit is the consolidation step. ConsolidateFindingScore takes the max score per detector and then averages across detectors, so a commit with just a Co-Authored-By trailer scores 85, but the same commit with an additional tool mention in the message body drops to 52.5. Adding corroborating evidence shouldn't lower the score. #12 was heading toward something additive (SpamAssassin-style) rather than an average — probably worth pinning that down on the issue before reworking the code.

Yes the max part is intentional, and this scoring indeed should be additive. But I guess we also need to have better weight defaults to avoid the score drops. In your case, score drops to 52.5 because both detectors get equal weights by default so 85x0.5 (trailer) + 20x0.5 (toolmention) = 52.5. I've used weights to normalize the score so that it always stays between 0 and 100 to automatically adjust for new detectors in future. Could you try with custom weights via cli? disclosure scan --weights=trailer=0.9,toolmention=0.1. Please try it and let me know your feedback :)

Related: the repo-wide OverallScore pools every finding from every commit into one max-per-detector average, so for a 500-commit range with one AI commit it just reports that one commit's score. I'm not sure a single number across a range is meaningful; the per-commit score is the useful bit.

Yes currently overall score is based on weighted average of findings across all commits. Makes sense, I'll update it to show score per commit (it's already in there just not using it yet).

Will also resolve the other 6 points (structural and minor) along with these changes. Thanks

[andrew]

Tried it. With --weights=trailer=0.9,toolmention=0.1, trailer+toolmention comes out to 78.5 (85×0.9 + 20×0.1), which is better than the 52.5 default, but it's still below the 85 that trailer-alone gets. Any weighted mean over the present detectors has that property: a weaker corroborating signal pulls the score toward itself.

I'd rather the consolidated score be max(perDetectorScores). Strongest signal wins, extra findings can't lower it, and it stays in 0–100 without needing weight tuning. The per-detector map you're already returning covers anyone who wants the breakdown.

Can revisit an additive scheme from #12 later if max turns out to be too coarse.

[omkar-foss]

I'd rather the consolidated score be max(perDetectorScores). Strongest signal wins, extra findings can't lower it, and it stays in 0–100 without needing weight tuning. The per-detector map you're already returning covers anyone who wants the breakdown.

Thanks for trying it out. I'll update this to use max, then let's try it out again. Yes agreed, if that too doesn't work well then we can revise to just have simple additive scoring.