Skip to content

Fix header-check logic flow and use -eq 0 for row count in run_sparql_metatest.sh#563

Merged
turbomam merged 5 commits into
mainfrom
copilot/fix-header-check-logic
Jun 29, 2026
Merged

Fix header-check logic flow and use -eq 0 for row count in run_sparql_metatest.sh#563
turbomam merged 5 commits into
mainfrom
copilot/fix-header-check-logic

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

The ontology-header-check section had two issues: the real metpo.owl check ran independently of whether the fixture proof-of-failure passed (confusing logic, misleading fail flag state), and used -le 0 instead of -eq 0 for a count that can never be negative.

Changes

  • Nested real OWL check inside fixture-success branch — the metpo.owl query now only runs when the fixture matched ≥1 rows. If the fixture check fails, the real-OWL check is skipped with an explicit message, and fail=1 is set once at the end of the else branch.
  • -le 0-eq 0 — row counts are never negative; this matches the -ge 1 pattern used elsewhere in the script.
if [ "$hrows" -ge 1 ]; then
  echo "OK   ontology-header-check.sparql matched $hrows fixture violation(s)"
  if [ -f "$REAL_OWL" ]; then
    ...
    if [ "$rrows" -eq 0 ]; then   # was -le 0
      echo "OK   ..."
    else
      fail=1
    fi
  else
    fail=1
  fi
else
  echo "::error::ontology-header-check.sparql matched 0 rows against the fixture: the check is a silent no-op."
  echo "Skipping check against metpo.owl because the fixture proof-of-failure did not pass."
  fail=1
fi
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The fail variable is set to 1 on line 82 but the script doesn't exit immediately. If the header check fails on line 82, execution continues to line 85 where it attempts to check the real OWL file. If that check passes, the script will still exit with code 1 due to the fail flag, but the logic flow is confusing. Consider adding an explicit exit or restructuring the checks so that the fail flag is evaluated consistently at the end of all checks.","fixFiles":[{"filePath":"tests/sparql_qc/run_sparql_metatest.sh","diff":"diff --git a/tests/sparql_qc/run_sparql_metatest.sh b/tests/sparql_qc/run_sparql_metatest.sh\n--- a/tests/sparql_qc/run_sparql_metatest.sh\n+++ b/tests/sparql_qc/run_sparql_metatest.sh\n@@ -77,23 +77,24 @@\n hrows=$(($(wc -l < \"$OUT/header-fixture.tsv\") - 1))\n if [ \"$hrows\" -ge 1 ]; then\n   echo \"OK   ontology-header-check.sparql matched $hrows fixture violation(s)\"\n-else\n-  echo \"::error::ontology-header-check.sparql matched 0 rows against the fixture: the check is a silent no-op.\"\n-  fail=1\n-fi\n \n-if [ -f \"$REAL_OWL\" ]; then\n-  robot query --input \"$REAL_OWL\" --query \"$HEADER_Q\" \"$OUT/header-real.tsv\"\n-  rrows=$(($(wc -l < \"$OUT/header-real.tsv\") - 1))\n-  if [ \"$rrows\" -le 0 ]; then\n-    echo \"OK   ontology-header-check.sparql found no problems in metpo.owl\"\n+  if [ -f \"$REAL_OWL\" ]; then\n+    robot query --input \"$REAL_OWL\" --query \"$HEADER_Q\" \"$OUT/header-real.tsv\"\n+    rrows=$(($(wc -l < \"$OUT/header-real.tsv\") - 1))\n+    if [ \"$rrows\" -le 0 ]; then\n+      echo \"OK   ontology-header-check.sparql found no problems in metpo.owl\"\n+    else\n+      echo \"::error::the ontology header in metpo.owl has $rrows metadata problem(s):\"\n+      cat \"$OUT/header-real.tsv\"\n+      fail=1\n+    fi\n   else\n-    echo \"::error::the ontology header in metpo.owl has $rrows metadata problem(s):\"\n-    cat \"$OUT/header-real.tsv\"\n+    echo \"::error::expected released ontology at $REAL_OWL but it is missing.\"\n     fail=1\n   fi\n else\n-  echo \"::error::expected released ontology at $REAL_OWL but it is missing.\"\n+  echo \"::error::ontology-header-check.sparql matched 0 rows against the fixture: the check is a silent no-op.\"\n+  echo \"Skipping check against metpo.owl because the fixture proof-of-failure did not pass.\"\n   fail=1\n fi\n \n"}]},{"message":"The condition uses -le 0 (less than or equal to zero) when checking if no rows were returned. Since row counts cannot be negative, use -eq 0 instead for clarity and to match the pattern used elsewhere in the script (e.g., line 48 uses -ge 1).","fixFiles":[{"filePath":"tests/sparql_qc/run_sparql_metatest.sh","diff":"diff --git a/tests/sparql_qc/run_sparql_metatest.sh b/tests/sparql_qc/run_sparql_metatest.sh\n--- a/tests/sparql_qc/run_sparql_metatest.sh\n+++ b/tests/sparql_qc/run_sparql_metatest.sh\n@@ -85,7 +85,7 @@\n if [ -f \"$REAL_OWL\" ]; then\n   robot query --input \"$REAL_OWL\" --query \"$HEADER_Q\" \"$OUT/header-real.tsv\"\n   rrows=$(($(wc -l < \"$OUT/header-real.tsv\") - 1))\n-  if [ \"$rrows\" -le 0 ]; then\n+  if [ \"$rrows\" -eq 0 ]; then\n     echo \"OK   ontology-header-check.sparql found no problems in metpo.owl\"\n   else\n     echo \"::error::the ontology header in metpo.owl has $rrows metadata problem(s):\"\n"}]}]

Copilot AI changed the title [WIP] Fix header check logic in run_sparql_metatest.sh Fix header-check logic flow and use -eq 0 for row count in run_sparql_metatest.sh Jun 29, 2026
Copilot AI requested a review from turbomam June 29, 2026 14:49
@turbomam turbomam marked this pull request as ready for review June 29, 2026 14:52
Copilot AI review requested due to automatic review settings June 29, 2026 14:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the control flow and clarity of the ontology header “meta-test” in run_sparql_metatest.sh, ensuring the real metpo.owl validation only runs when the fixture proof-of-failure succeeds, and tightening the row-count comparison to reflect expected non-negative counts.

Changes:

  • Nest the real metpo.owl header query under the successful fixture-violation check, and explicitly skip the real-OWL check when the fixture returns 0 rows.
  • Replace the -le 0 comparison with -eq 0 for the real-OWL row count check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

turbomam and others added 2 commits June 29, 2026 10:59
The header-real query writes an empty TSV when metpo.owl has no header
problems, so 'wc -l - 1' yields -1, not 0. With the new '-eq 0' check that
sent a clean ontology down the error branch ('has -1 metadata problem(s)')
and failed CI. Clamp the count to 0 before the comparison; this keeps the
clearer -eq 0 form and fixes the regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@turbomam

Copy link
Copy Markdown
Contributor

Pushed a fix for the failing sparql-metatest. Root cause: the -eq 0 change (from the AI review) breaks on this script's row-counting idiom. rrows=$(( $(wc -l < header-real.tsv) - 1 )), and robot query writes an empty TSV (no header row) when there are no results, so a clean metpo.owl gives wc -l=0 and rrows=-1, not 0. The old -le 0 absorbed that; -eq 0 sent it to the error branch ("has -1 metadata problem(s)"). I clamp the count to 0 before the comparison, which keeps the clearer -eq 0 and fixes the regression. Verified locally: the metatest now passes and reports "found no problems in metpo.owl". The control-flow restructuring in this PR is fine and unchanged.

@turbomam turbomam merged commit 1dcceef into main Jun 29, 2026
15 checks passed
@turbomam turbomam deleted the copilot/fix-header-check-logic branch June 29, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants