Show the per-metric comparison table by default in checkQorMetrics.py

docs/contrib/Metrics.md

@@ -109,7 +109,7 @@ To check every run under `reports/` at once, call the script directly:
 cd OpenROAD-flow-scripts/flow
 ./util/checkQorMetrics.py                     # sweep, log in reports/inline-check.log
 ./util/checkQorMetrics.py --base-commit <sha> # pin the baseline build
-./util/checkQorMetrics.py --verbose           # show passing rules and absent metrics
+./util/checkQorMetrics.py --quiet             # leave out the per-metric table
 ```
 
 If the check reports a failed rule, review it to make sure the change in

flow/test/test_checkQorMetrics.py

@@ -411,9 +411,9 @@ def test_fail_lists_the_failed_rule(self):
         failed = {
             "metricName": "finish__timing__setup__ws",
             "status": "fail",
-            "baselineValue": 2,
-            "currentValue": 1.5,
-            "limit": 1.9,
+            "baseValue": 2,
+            "targetValue": 1.5,
+            "thresholdValue": 1.9,
             "deltaPct": -25,
         }
         code, output, _ = self.run_check(
@@ -424,6 +424,87 @@ def test_fail_lists_the_failed_rule(self):
         self.assertIn("-25.00", output)
         self.assertIn("QoR check: FAIL", output)
 
+    def table_reply(self):
+        passed = {
+            "metricName": "synth__design__instance__area",
+            "status": "pass",
+            "baseValue": 100,
+            "targetValue": 101,
+            "thresholdValue": 110,
+            "deltaPct": 1,
+        }
+        failed = {
+            "metricName": "finish__timing__setup__ws",
+            "status": "fail",
+            "baseValue": 2,
+            "targetValue": 1.5,
+            "thresholdValue": 1.9,
+            "deltaPct": -25,
+        }
+        absent = [
+            {"metricName": name, "status": "missing_from_baseline"}
+            for name in ("route__wirelength", "cts__clock__skew")
+        ]
+        return Dashboard(
+            response(status="fail", failed=1, metrics=[passed, failed, *absent])
+        )
+
+    def table_rows(self, output):
+        return [line for line in output.splitlines() if line.startswith("| ")]
+
+    def test_table_shows_every_evaluated_metric_with_its_status(self):
+        _, output, _ = self.run_check(dashboard=self.table_reply())
+        rows = self.table_rows(output)
+        self.assertEqual(len(rows), 4)
+        self.assertTrue(rows[0].rstrip("| ").endswith("Status"))
+        self.assertIn("finish__timing__setup__ws", rows[2])
+        self.assertTrue(rows[2].rstrip().endswith("| FAIL   |"))
+        self.assertIn("synth__design__instance__area", rows[3])
+        self.assertTrue(rows[3].rstrip().endswith("| PASS   |"))
+        self.assertEqual(len({len(row) for row in rows}), 1)
+
+    def test_passing_table_has_no_error_line(self):
+        passed = {
+            "metricName": "synth__design__instance__area",
+            "status": "pass",
+            "baseValue": 100,
+            "targetValue": 101,
+            "thresholdValue": 110,
+            "deltaPct": 1,
+        }
+        code, output, _ = self.run_check(
+            dashboard=Dashboard(response(metrics=[passed]))
+        )
+        self.assertEqual(code, 0)
+        self.assertEqual(len(self.table_rows(output)), 3)
+        self.assertFalse(
+            [line for line in output.splitlines() if line.startswith("[ERROR]")]
+        )
+
+    def test_absent_metrics_are_named_not_tabled(self):
+        _, output, _ = self.run_check(dashboard=self.table_reply())
+        self.assertIn(
+            "[WARN] 2 ruled metric(s) absent from the baseline build, not evaluated:\n"
+            "  cts__clock__skew\n"
+            "  route__wirelength\n",
+            output,
+        )
+        for row in self.table_rows(output):
+            self.assertNotIn("route__wirelength", row)
+
+    def test_quiet_leaves_out_only_the_table(self):
+        for flag in ("--quiet", "-q"):
+            code, output, _ = self.run_check(flag, dashboard=self.table_reply())
+            self.assertEqual(code, 1)
+            self.assertEqual(self.table_rows(output), [])
+            self.assertIn("QoR regression detected", output)
+            self.assertIn("  cts__clock__skew", output)
+            self.assertIn("QoR check: FAIL", output)
+
+    def test_verbose_is_rejected(self):
+        with self.assertRaises(SystemExit), contextlib.redirect_stderr(io.StringIO()):
+            self.run_check("--verbose")
+
     def test_latest_master_warns_with_a_commit(self):
         _, output, _ = self.run_check(
             dashboard=Dashboard(response(strategy="latest-master"))

flow/util/checkQorMetrics.py

@@ -39,6 +39,10 @@
 its checkout has no branch. A local run of a private platform does not need it:
 DASHBOARD_API_KEY alone lets the default pipelines resolve it.
 
+Each run's report has a table of every metric a rule evaluated, with its
+baseline, current value, limit, change and PASS/FAIL status (--quiet leaves
+the table out), and names each ruled metric absent from the baseline build.
+
 Exit status: 1 when the dashboard reports a failed rule for any checked run,
 or when a metadata.json is missing, malformed, or holds no numeric metric.
 0 otherwise. A run the dashboard could not judge -- no baseline, nothing
@@ -95,7 +99,7 @@
 
 # NAME_W must match the first field width in ROW_FMT.
 NAME_W = 46
-ROW_FMT = "| {:46} | {:>12} | {:>12} | {:>12} | {:>7} |"
+ROW_FMT = "| {:46} | {:>12} | {:>12} | {:>12} | {:>7} | {:6} |"
 BANNER = "-" * 79
 
 
@@ -446,7 +450,7 @@ def dropped_lines(result):
     ]
 
 
-def render_run(result, verbose):
+def render_run(result, quiet):
     """Builds the report lines for one run."""
     identity = f"{result['platform']}/{result['design']}/{result['variant']}"
     lines = [BANNER]
@@ -527,13 +531,14 @@ def render_run(result, verbose):
         )
 
     metrics = response.get("metrics", [])
-    shown = metrics if verbose else [m for m in metrics if m.get("status") == "fail"]
-    shown = [m for m in shown if m.get("status") != "missing_from_baseline"]
+    shown = [m for m in metrics if m.get("status") != "missing_from_baseline"]
 
-    if shown:
-        lines.append(ROW_FMT.format("Metric", "Baseline", "Current", "Limit", "Delta%"))
+    if shown and not quiet:
+        lines.append(
+            ROW_FMT.format("Metric", "Baseline", "Current", "Limit", "Delta%", "Status")
+        )
         lines.append(
-            ROW_FMT.format("-" * NAME_W, "-" * 12, "-" * 12, "-" * 12, "-" * 7)
+            ROW_FMT.format("-" * NAME_W, "-" * 12, "-" * 12, "-" * 12, "-" * 7, "-" * 6)
         )
         for metric in sorted(shown, key=lambda m: m.get("metricName", "")):
             delta = metric.get("deltaPct")
@@ -544,6 +549,7 @@ def render_run(result, verbose):
                     fmt_value(metric.get("targetValue")),
                     fmt_value(metric.get("thresholdValue")),
                     fmt_delta(delta),
+                    str(metric.get("status", "?")).upper(),
                 )
             )
 
@@ -560,22 +566,17 @@ def render_run(result, verbose):
                 f"limit {fmt_exact_value(metric.get('thresholdValue'))})"
             )
 
-    # A design can have hundreds of absent metrics, so name them only on request.
     absent = sorted(
         metric.get("metricName", "?")
         for metric in metrics
         if metric.get("status") == "missing_from_baseline"
     )
-    if absent and verbose:
-        lines.append(
-            "[WARN] Rules not evaluated, metric absent from the baseline build: "
-            + ", ".join(absent)
-        )
-    elif absent:
+    if absent:
         lines.append(
-            f"[WARN] Rules not evaluated for {len(absent)} metric(s) absent from "
-            "the baseline build. Use --verbose to list them."
+            f"[WARN] {len(absent)} ruled metric(s) absent from the baseline "
+            "build, not evaluated:"
         )
+        lines.extend(f"  {name}" for name in absent)
 
     # A rule that fell back reports a different threshold than the one it was
     # configured with, so surface it. thresholdPct is deliberately not shown in
@@ -779,10 +780,11 @@ def parse_args(argv):
         help="Concurrent requests. Default: 8.",
     )
     parser.add_argument(
-        "--verbose",
+        "--quiet",
+        "-q",
         action="store_true",
-        help="Report every rule-checked metric, not only the failing ones, "
-        "and name the metrics absent from the baseline build.",
+        help="Leave out the per-metric comparison table. Failed rules, warnings "
+        "and the verdict are still reported.",
     )
     args = parser.parse_args(argv)
     if args.metadata and not (args.platform and args.design):
@@ -948,7 +950,7 @@ def run_check(argv=None):
 
     body = []
     for result in results:
-        body.extend(render_run(result, args.verbose))
+        body.extend(render_run(result, args.quiet))
     body.extend(summarize(results))
 
     for line in body: