Skip to content

fix: reword misleading 'stale' warning in info command#510

Merged
carlos-alm merged 4 commits intomainfrom
fix/info-stale-warning
Mar 19, 2026
Merged

fix: reword misleading 'stale' warning in info command#510
carlos-alm merged 4 commits intomainfrom
fix/info-stale-warning

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • The codegraph info command showed (binary reports X.Y.Z — stale) when the npm package version differed from the binary's compiled-in version, even though the engine works correctly
  • Reworded to (binary built as X.Y.Z, cosmetic only) to clarify this is a harmless version-string mismatch, not a broken engine

Test plan

  • Run codegraph info with a native engine where package and binary versions differ — confirm new wording appears
  • Run codegraph info with matching versions — confirm no parenthetical note

The native version mismatch between npm package and binary is cosmetic
— the package version is authoritative. Replace alarming 'stale' label
with 'cosmetic only' to avoid confusion.
@claude
Copy link

claude bot commented Mar 19, 2026

Claude finished @carlos-alm's task —— View job


PR Review Analysis

Todo List:

  • Read the modified file to understand the change
  • Perform Phase 1: Root Cause Analysis
  • Perform Phase 2: Configuration Change Detection
  • Perform Phase 3: Test Integrity Check
  • Perform Phase 4: Code Quality Assessment
  • Perform Phase 5: Backlog Compliance Evaluation
  • Provide final assessment with rating and recommendation

Root Cause Analysis

Problem Being Solved: Users were confused by the word "stale" in codegraph info output when npm package version differed from binary's compiled-in version, thinking their engine was broken.

Why This Approach: Rewording from "stale" to "cosmetic only" directly addresses the confusion without changing functionality. The binary works correctly regardless of version string mismatch.

Risk Assessment: Extremely low risk. Pure message improvement with no behavioral changes.

Backlog Compliance

  • Zero-dep: ✓ — No new dependencies
  • Foundation-aligned: ✓ — Fully aligned with all principles
  • Problem-fit: 2 — Nice UX improvement but doesn't address the core "AI agents waste tokens" problem
  • Breaking: No — Pure message change, no API/CLI contract changes
  • Tier: 1

Critical Concerns

None. This is a straightforward UX improvement.

Code Quality Assessment

  • Change location: src/cli/commands/info.js:28
  • Scope: Single line message change from "(binary reports ${knownBinaryVersion} — stale)" to "(binary built as ${knownBinaryVersion}, cosmetic only)"
  • Context preservation: The logic remains identical - only the user-facing message is improved
  • Clarity improvement: "cosmetic only" is much clearer than "stale" for describing a harmless version mismatch

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a clean, well-targeted fix that improves user experience without any risk. The commit message is clear, the change is minimal and focused, and it directly addresses reported user confusion. While it doesn't solve the core AI agent problem (hence not 5 stars), it's exactly the kind of polish that makes codegraph more professional and user-friendly.

The PR author correctly identified a UX pain point and applied the most surgical fix possible. The test plan is appropriate for a diagnostic message change.


@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR rewrites one diagnostic string in codegraph info: the parenthetical (binary reports X.Y.Z — stale) is replaced with (binary built as X.Y.Z, engine loaded OK) to clarify that a version-string mismatch between the npm wrapper and the compiled binary does not mean the engine is broken.

Key observations:

  • The only change is a user-facing string in src/cli/commands/info.js — no logic, no behaviour, no data is affected.
  • The new wording is accurate: reaching line 28 means loadNative() succeeded and native.engineVersion() returned a non-unknown value, so "engine loaded OK" is a true statement scoped to what the code has actually verified.
  • The previous review thread flagged "cosmetic only" as over-promising; the author addressed it by switching to "engine loaded OK", which correctly limits the claim to the load step without asserting all mismatches are harmless.
  • No tests need updating for a display-string change in a diagnostics command.

Confidence Score: 5/5

  • Safe to merge — pure cosmetic string change with no logic impact.
  • The entire diff is a single string literal reword. No control flow, data handling, or API surface is touched. The new wording is accurate given the surrounding code, and prior review feedback has been incorporated.
  • No files require special attention.

Important Files Changed

Filename Overview
src/cli/commands/info.js Single display-string reword: (binary reports X — stale)(binary built as X, engine loaded OK). No logic changes; the new wording accurately reflects what the code has verified (that loadNative() succeeded and engineVersion() returned a value).

Sequence Diagram

sequenceDiagram
    participant User
    participant InfoCmd as info command
    participant Native as infrastructure/native.js
    participant Parser as domain/parser.js

    User->>InfoCmd: codegraph info
    InfoCmd->>Native: isNativeAvailable()
    Native-->>InfoCmd: true / false
    InfoCmd->>Parser: getActiveEngine({ engine })
    Parser-->>InfoCmd: { name, version }

    alt native is available
        InfoCmd->>Native: loadNative()
        Native-->>InfoCmd: native addon (loaded OK)
        InfoCmd->>Native: native.engineVersion()
        Native-->>InfoCmd: binaryVersion string
        InfoCmd->>Native: getNativePackageVersion()
        Native-->>InfoCmd: pkgVersion string

        alt pkgVersion !== binaryVersion
            InfoCmd->>User: "Native version: {pkgVersion} (binary built as {binaryVersion}, engine loaded OK)"
        else versions match
            InfoCmd->>User: "Native version: {pkgVersion}"
        end
    end

    InfoCmd->>InfoCmd: try open DB, read build metadata
    InfoCmd->>User: build metadata (engine, version, built_at) + stale warnings if applicable
Loading

Last reviewed commit: "Merge branch 'fix/in..."

if (pkgVersion && knownBinaryVersion && pkgVersion !== knownBinaryVersion) {
console.log(
` Native version: ${pkgVersion} (binary reports ${knownBinaryVersion} — stale)`,
` Native version: ${pkgVersion} (binary built as ${knownBinaryVersion}, cosmetic only)`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 "cosmetic only" may over-reassure in edge cases

The phrase cosmetic only implies the engine is always unaffected by the version mismatch. However, if the npm wrapper package and the compiled binary diverge across a breaking native-API change (e.g., a napi-rs interface update), the mismatch could affect runtime behaviour, not just a display string.

A slightly more precise label would scope the claim to what the code actually knows — that the binary loaded without error — without asserting that all mismatches are harmless:

Suggested change
` Native version: ${pkgVersion} (binary built as ${knownBinaryVersion}, cosmetic only)`,
` Native version: ${pkgVersion} (binary built as ${knownBinaryVersion}, engine loaded OK)`,

This keeps the friendly tone ("not a crash") while avoiding a blanket guarantee that the PR description itself doesn't establish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — changed to engine loaded OK as suggested, which scopes the claim to what the code actually verifies (that the binary loaded without error) without asserting all mismatches are harmless.

…smatch

Scopes the reassurance to what the code actually verifies — that the
binary loaded without error — rather than implying all mismatches are
harmless.
@carlos-alm
Copy link
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit a5e141a into main Mar 19, 2026
17 checks passed
@carlos-alm carlos-alm deleted the fix/info-stale-warning branch March 19, 2026 05:39
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant