feat(core): add stable finding fingerprints to scanner output#175
Merged
Conversation
Implements ADR 007. Adds fingerprint: string to FlagUsage using schema: launchdarkly:callType:flagKey:normalizedFilePath Stable across line-number changes. Dynamic keys get a sequential index. Part of the v1.0.0 milestone. Signed-off-by: Krishan Kant Sharma <krishansharma0327@gmail.com>
📝 WalkthroughWalkthroughAdds a ChangesStable FlagUsage Fingerprint
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Deploying flaglint with
|
| Latest commit: |
5ca1009
|
| Status: | ⚡️ Build in progress... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/readiness/readiness.test.ts (1)
23-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
makeUsagefingerprint should be derived after applying overrides.Line 31 hardcodes a fingerprint, while overrides are merged in the same object. Overriding
flagKey,callType, orfilewithout also overridingfingerprintcreates invalid fixture state.💡 Suggested fix
function makeUsage(overrides: Partial<FlagUsage> = {}): FlagUsage { - return { + const usage = { flagKey: "my-flag", isDynamic: false, file: "src/app.ts", line: 1, column: 0, callType: "boolVariation", - fingerprint: "launchdarkly:boolVariation:my-flag:src/app.ts", stalenessSignals: [], ...overrides, }; + return { + ...usage, + fingerprint: + overrides.fingerprint ?? + `launchdarkly:${usage.callType}:${usage.flagKey}:${usage.file}`, + }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/readiness/readiness.test.ts` around lines 23 - 34, The makeUsage function hardcodes the fingerprint value before applying overrides, which means when flagKey, callType, or file are overridden, the fingerprint becomes inconsistent with the actual values. Refactor makeUsage to first merge the default object with overrides, then compute the fingerprint dynamically based on the final values of flagKey, callType, and file. This ensures the fingerprint is always valid and reflects the actual properties of the FlagUsage object being created.src/migrator/tests/migrator.test.ts (1)
22-33: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
makeUsagecan emit mismatched fixture data when overrides change key/type/path.On Line 30,
fingerprintis fixed to legacy defaults, but Line 32 applies...overridesafterward. If a test overridesflagKey,callType, orfilewithout overridingfingerprint, the helper returns an inconsistentFlagUsage.💡 Suggested fix
function makeUsage(overrides: Partial<FlagUsage>): FlagUsage { - return { + const usage = { flagKey: "legacy-flag", isDynamic: false, file: "src/legacy.ts", line: 3, column: 4, callType: "variation", - fingerprint: "launchdarkly:variation:legacy-flag:src/legacy.ts", stalenessSignals: [], ...overrides, }; + return { + ...usage, + fingerprint: + overrides.fingerprint ?? + `launchdarkly:${usage.callType}:${usage.flagKey}:${usage.file}`, + }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/migrator/tests/migrator.test.ts` around lines 22 - 33, The makeUsage helper function has a fixture data consistency issue where the hardcoded fingerprint on line 30 does not get updated when overrides modify flagKey, callType, or file. Move the fingerprint computation to occur after the overrides are applied so it is dynamically constructed based on the actual values of flagKey, callType, and file in the returned object. This ensures that whenever these properties are overridden in tests, the fingerprint remains consistent with the actual flag usage data.
🧹 Nitpick comments (2)
src/validator/tests/validate.test.ts (1)
15-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer shared fingerprint generation in
makeScanResultdefaults.Line 26 reconstructs the fingerprint string manually. Reusing
generateFingerprintkeeps validator fixtures aligned with scanner/migrator rules (notably normalization/coercion behavior) as the schema evolves.💡 Suggested refactor
+import { generateFingerprint } from "../../scanner/fingerprint"; callType, - fingerprint: u.fingerprint ?? `launchdarkly:${callType}:${flagKey}:${file}`, + fingerprint: u.fingerprint ?? generateFingerprint(flagKey, callType, file), stalenessSignals: u.stalenessSignals ?? [],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/validator/tests/validate.test.ts` around lines 15 - 27, The test fixture in the map function that constructs the default FlagUsage objects is manually building the fingerprint string instead of using the shared generateFingerprint utility function. Replace the manual fingerprint string construction (the line with the template literal launchdarkly:${callType}:${flagKey}:${file}) with a call to generateFingerprint, passing the appropriate parameters (flagKey, callType, and file) to ensure test fixtures use the same fingerprint generation logic and normalization behavior as the actual scanner/migrator implementation.src/migrator/tests/migrator.test.ts (1)
49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
generateFingerprintinmakeResultto mirror production behavior exactly.Line 56 re-implements the format inline. That bypasses shared rules (path normalization and key coercion) already defined in
src/scanner/fingerprint.ts, so test fixtures can drift from runtime behavior.💡 Suggested refactor
+import { generateFingerprint } from "../../scanner/fingerprint"; usages: migrationInventory.map((item) => ({ flagKey: item.staticFlagKey ?? (item.isDynamic ? "dynamic" : "*"), isDynamic: item.isDynamic, file: item.file, line: item.line, column: item.column, callType: item.launchDarklyMethod, - fingerprint: `launchdarkly:${item.launchDarklyMethod}:${item.staticFlagKey ?? (item.isDynamic ? "dynamic" : "*")}:${item.file}`, + fingerprint: generateFingerprint( + item.staticFlagKey ?? (item.isDynamic ? "dynamic" : "*"), + item.launchDarklyMethod, + item.file + ), stalenessSignals: [], })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/migrator/tests/migrator.test.ts` around lines 49 - 57, The fingerprint field in the usages mapping within makeResult is constructing the fingerprint value inline using a template string instead of using the shared generateFingerprint function from src/scanner/fingerprint.ts. Replace the inline fingerprint template string construction with a call to the generateFingerprint function, passing the appropriate parameters (item.launchDarklyMethod, item.staticFlagKey, item.isDynamic, and item.file) to ensure the test behavior respects the same path normalization and key coercion rules that are applied at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/migrator/index.ts`:
- Around line 101-110: The generateFingerprint call in buildEvidenceItem does
not include the dynamicIndex parameter for dynamic items, causing multiple
dynamic usages in the same file and method to produce identical fingerprints.
Fix this by passing item.dynamicIndex (or the appropriate dynamic index field)
to the generateFingerprint function when item.isDynamic is true, ensuring each
distinct dynamic usage receives a unique fingerprint that matches the
scanner-side contract.
---
Outside diff comments:
In `@src/migrator/tests/migrator.test.ts`:
- Around line 22-33: The makeUsage helper function has a fixture data
consistency issue where the hardcoded fingerprint on line 30 does not get
updated when overrides modify flagKey, callType, or file. Move the fingerprint
computation to occur after the overrides are applied so it is dynamically
constructed based on the actual values of flagKey, callType, and file in the
returned object. This ensures that whenever these properties are overridden in
tests, the fingerprint remains consistent with the actual flag usage data.
In `@src/readiness/readiness.test.ts`:
- Around line 23-34: The makeUsage function hardcodes the fingerprint value
before applying overrides, which means when flagKey, callType, or file are
overridden, the fingerprint becomes inconsistent with the actual values.
Refactor makeUsage to first merge the default object with overrides, then
compute the fingerprint dynamically based on the final values of flagKey,
callType, and file. This ensures the fingerprint is always valid and reflects
the actual properties of the FlagUsage object being created.
---
Nitpick comments:
In `@src/migrator/tests/migrator.test.ts`:
- Around line 49-57: The fingerprint field in the usages mapping within
makeResult is constructing the fingerprint value inline using a template string
instead of using the shared generateFingerprint function from
src/scanner/fingerprint.ts. Replace the inline fingerprint template string
construction with a call to the generateFingerprint function, passing the
appropriate parameters (item.launchDarklyMethod, item.staticFlagKey,
item.isDynamic, and item.file) to ensure the test behavior respects the same
path normalization and key coercion rules that are applied at runtime.
In `@src/validator/tests/validate.test.ts`:
- Around line 15-27: The test fixture in the map function that constructs the
default FlagUsage objects is manually building the fingerprint string instead of
using the shared generateFingerprint utility function. Replace the manual
fingerprint string construction (the line with the template literal
launchdarkly:${callType}:${flagKey}:${file}) with a call to generateFingerprint,
passing the appropriate parameters (flagKey, callType, and file) to ensure test
fixtures use the same fingerprint generation logic and normalization behavior as
the actual scanner/migrator implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b068fcd3-9db7-4cd1-a64d-83254cfb2975
📒 Files selected for processing (9)
src/migrator/index.tssrc/migrator/tests/migrator.test.tssrc/readiness/readiness.test.tssrc/reporter/tests/reporter.test.tssrc/scanner/fingerprint.tssrc/scanner/index.tssrc/scanner/tests/fingerprint.test.tssrc/types.tssrc/validator/tests/validate.test.ts
Comment on lines
+101
to
+110
| const usageFlagKey = item.staticFlagKey ?? (item.isDynamic ? "dynamic" : "*"); | ||
| return { | ||
| usage: { | ||
| flagKey, | ||
| flagKey: usageFlagKey, | ||
| isDynamic: item.isDynamic, | ||
| file: item.file, | ||
| line: item.line, | ||
| column: item.column, | ||
| callType: item.launchDarklyMethod, | ||
| fingerprint: generateFingerprint(flagKey, item.launchDarklyMethod, item.file), | ||
| fingerprint: generateFingerprint(usageFlagKey, item.launchDarklyMethod, item.file), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Dynamic-key fingerprints in migrator can collide.
buildEvidenceItem never passes dynamicIndex to generateFingerprint, so multiple dynamic items in the same file/method can produce identical fingerprints. Scanner-side dynamic usages already add a per-file index, so this creates a contract mismatch and can collapse distinct evidence findings.
🔧 Proposed fix
-function buildEvidenceItem(item: MigrationInventoryItem): MigrationItem {
+function buildEvidenceItem(item: MigrationInventoryItem, dynamicIndex?: number): MigrationItem {
const flagLabel = item.staticFlagKey ?? item.flagKeyExpression ?? "*";
@@
const usageFlagKey = item.staticFlagKey ?? (item.isDynamic ? "dynamic" : "*");
return {
usage: {
flagKey: usageFlagKey,
isDynamic: item.isDynamic,
file: item.file,
line: item.line,
column: item.column,
callType: item.launchDarklyMethod,
- fingerprint: generateFingerprint(usageFlagKey, item.launchDarklyMethod, item.file),
+ fingerprint: generateFingerprint(
+ usageFlagKey,
+ item.launchDarklyMethod,
+ item.file,
+ dynamicIndex
+ ),
stalenessSignals: [],
},
@@
export function analyze(result: ScanResult): MigrationAnalysis {
const inventoryItems = inventoryFrom(result);
- const items = inventoryItems.map(buildEvidenceItem);
+ const dynamicIndexByFile = new Map<string, number>();
+ const items = inventoryItems.map((item) => {
+ if (!item.isDynamic) return buildEvidenceItem(item);
+ const idx = dynamicIndexByFile.get(item.file) ?? 0;
+ dynamicIndexByFile.set(item.file, idx + 1);
+ return buildEvidenceItem(item, idx);
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/migrator/index.ts` around lines 101 - 110, The generateFingerprint call
in buildEvidenceItem does not include the dynamicIndex parameter for dynamic
items, causing multiple dynamic usages in the same file and method to produce
identical fingerprints. Fix this by passing item.dynamicIndex (or the
appropriate dynamic index field) to the generateFingerprint function when
item.isDynamic is true, ensuring each distinct dynamic usage receives a unique
fingerprint that matches the scanner-side contract.
This was referenced Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ADR 007. Adds fingerprint field to FlagUsage (launchdarkly:callType:flagKey:normalizedFilePath). Stable across line-number changes. Foundation for baseline mode. Part of the v1.0.0 milestone.
Summary by CodeRabbit
New Features
Tests