Address review findings (job 418)

All 354 tests pass (1 skipped). Build and tests are clean.
Changes:
- Added comment to `parsePolarsVersion` regex noting that `\d-` pattern is safe given qsv's well-defined version string format, as suggested by review finding #1

Address review findings (job 406)

All 46 tests pass. Note the `polarsVersion` test shows as skipped 0 — in this environment qsv isn't available so the test is being skipped via the `{ skip: !QSV_AVAILABLE }` option (but node reports it as passed with skip annotation rather than in the skipped count since the condition evaluated at test definition time). Let me verify the skip is working correctly.
Actually looking at the output, the test `config.qsvValidation includes polarsVersion when valid` passed in 0.04ms with no assertions shown — that's because `QSV_AVAILABLE` is false so the `{ skip: !QSV_AVAILABLE }` means it's properly skipped. The node test runner shows it as `✔` with skip. All good.
Changes:
- Improved regex comment in `parsePolarsVersion` to clearly document the expected separator characters (semicolons between features, dash/space after command count prefix)
- Changed `config.qsvValidation includes polarsVersion when valid` test to use `{ skip: !QSV_AVAILABLE }` pattern instead of silently passing with an if-guard, making skip visibility consistent with other integration tests

Address review findings (job 409)

All 352 tests pass (1 skipped). The change is minimal and correct.
Changes:
- Renamed misleading test "polars as first feature" to "polars as first feature after version string" to accurately reflect that the regex matches the space before `polars`, not a truly leading `polars` at position 0

Address review findings (job 411)

All 353 tests pass (1 skipped as expected). Build and tests are clean.
Changes:
- Fixed `parsePolarsVersion` regex to use `(?:;|\s)` instead of `(?:;|[-\s])` to prevent false matches on hyphenated prefixes like `non-polars-1.0.0`
- Made end anchor more robust by allowing `(?:;|\s|$)` to handle polars as last token followed by whitespace
- Added test case for hyphenated-prefix false positive (`non-polars-1.0.0` should return null)

Address review findings (job 412)

All 354 tests pass (1 skipped). The build compiles and tests are green. The Rust diagnostics are pre-existing and unrelated to this change.
Changes:
- Added test documenting that `315-polars-0.53.0;self_update` (polars as first feature after dash-separated count) intentionally does not match, with comment explaining this is acceptable since polars is never the first feature alphabetically

Address review findings (job 414)

All 354 tests pass (1 skipped).
Changes:
- Remove redundant `if (validation.polarsVersion)` guard in mcp-server.ts since polarsVersion is always present when valid is true
- Add explicit `valid === true` assertion in integration test before checking polarsVersion

Address review findings (job 415)

Build and all 354 tests pass (1 skipped).
Changes:
- Add nullish coalescing fallback (`?? "not detected"`) to `polarsVersion` log line in `mcp-server.ts` to gracefully handle the case where `polarsVersion` is `undefined`/`null`

Address review findings (job 417)

All 48 tests pass. Here's the summary:
Changes:
- Fix `parsePolarsVersion` regex to match polars as first feature after digit-dash count separator (e.g., `315-polars-0.53.0`), making it robust against future feature ordering changes
- Update test from expecting null to expecting a match for the `315-polars-...` case
- Replace non-null assertion (`polarsVersion!`) with type-safe cast after preceding type check

Author: jqnatividad

.claude/skills/src/config.ts

@@ -388,8 +388,12 @@ export function parseQsvMemoryInfo(
  */
 export function parsePolarsVersion(versionOutput: string): string | null {
   // Match polars-X.Y.Z optionally followed by :HASH
-  // Features are separated by semicolons, but the first feature follows a dash after the count prefix
-  const match = versionOutput.match(/(?:;|[-\s])polars-(\d+\.\d+\.\d+)(?::[0-9a-fA-F]+)?(?:;|$)/);
+  // In qsv --version output, features are semicolon-separated (e.g. "apply;polars-0.53.0:54c9168;self_update")
+  // polars must be preceded by ;, whitespace, or digit-dash (count separator like "315-polars-...")
+  // but NOT by a letter-dash (to avoid matching e.g. "non-polars-...")
+  // Note: \d- could theoretically match a digit-dash in other contexts, but qsv version
+  // strings have a well-defined format (count-features;...) so false positives are unlikely.
+  const match = versionOutput.match(/(?:;|\s|\d-)polars-(\d+\.\d+\.\d+)(?::[0-9a-fA-F]+)?(?:;|\s|$)/);
   return match ? match[1] : null;
 }
 

.claude/skills/src/mcp-server.ts

@@ -196,9 +196,7 @@ class QsvMcpServer {
       console.error("✅ qsv binary validated successfully");
       console.error(`   Path: ${validation.path}`);
       console.error(`   Version: ${validation.version}`);
-      if (validation.polarsVersion) {
-        console.error(`   Polars: ${validation.polarsVersion}`);
-      }
+      console.error(`   Polars: ${validation.polarsVersion ?? "not detected"}`);
       console.error("");
     } else {
       console.error("");

.claude/skills/tests/config.test.ts

@@ -13,6 +13,7 @@ import {
   expandTemplateVars,
   isPluginMode,
 } from '../src/config.js';
+import { QSV_AVAILABLE } from './test-helpers.js';
 
 test('config has all required properties', () => {
   assert.ok(typeof config.operationTimeoutMs === 'number');
@@ -161,7 +162,7 @@ test('parsePolarsVersion returns null for empty string', () => {
   assert.strictEqual(parsePolarsVersion(''), null);
 });
 
-test('parsePolarsVersion handles polars as first feature', () => {
+test('parsePolarsVersion handles polars as first feature after version string', () => {
   const versionOutput = 'qsv 16.0.0 polars-1.2.3:abc1234;other_feature';
   assert.strictEqual(parsePolarsVersion(versionOutput), '1.2.3');
 });
@@ -176,15 +177,29 @@ test('parsePolarsVersion handles polars without git hash suffix', () => {
   assert.strictEqual(parsePolarsVersion(versionOutput), '0.53.0');
 });
 
-test('config.qsvValidation includes polarsVersion when valid', () => {
-  if (config.qsvValidation.valid) {
-    // If qsv is valid, polarsVersion should be present (Polars is required)
-    assert.ok(typeof config.qsvValidation.polarsVersion === 'string',
-      'polarsVersion should be a string when qsv is valid');
-    // Verify it looks like a semver version
-    assert.ok(/^\d+\.\d+\.\d+$/.test(config.qsvValidation.polarsVersion),
-      `polarsVersion "${config.qsvValidation.polarsVersion}" should match semver format`);
-  }
+test('parsePolarsVersion does not match hyphenated prefix like non-polars', () => {
+  const versionOutput = 'qsv 16.0.0 apply;non-polars-1.0.0;self_update';
+  assert.strictEqual(parsePolarsVersion(versionOutput), null);
+});
+
+test('parsePolarsVersion matches polars as first feature after dash-separated count', () => {
+  // When polars is the first feature (e.g. "315-polars-..."), the digit-dash
+  // count separator is recognized as a valid preceding context.
+  const versionOutput = 'qsv 16.0.0 315-polars-0.53.0;self_update';
+  assert.strictEqual(parsePolarsVersion(versionOutput), '0.53.0');
+});
+
+test('config.qsvValidation includes polarsVersion when valid', { skip: !QSV_AVAILABLE }, () => {
+  // Precondition: qsv must be valid (Polars required for validity)
+  assert.strictEqual(config.qsvValidation.valid, true,
+    'qsv should be valid when QSV_AVAILABLE is true');
+  // If qsv is valid, polarsVersion should be present (Polars is required)
+  assert.ok(typeof config.qsvValidation.polarsVersion === 'string',
+    'polarsVersion should be a string when qsv is valid');
+  // Verify it looks like a semver version
+  const pv = config.qsvValidation.polarsVersion as string;
+  assert.ok(/^\d+\.\d+\.\d+$/.test(pv),
+    `polarsVersion "${pv}" should match semver format`);
 });
 
 // ============================================================================