fix(load): silence skip-WARN for legitimate non-artifact YAML (REQ-139)

CHANGELOG.md

@@ -65,6 +65,17 @@
 
 ### Fixed
 
+- **REQ-139 — directory import no longer warns on legitimate non-artifact
+  YAML.** The generic-YAML directory load warned `[WARN] skipping <file>` for
+  *every* file it declined to load — including expected non-artifact YAML
+  under the source path (`bindings.yaml`, `feature-model.yaml`,
+  `variants/*.yaml`). On a real project that put a WARN line in front of every
+  command, burying the signal (reported in #353). The load-path WARN now keys
+  off the existing REQ-062 `SkipKind`: it fires only for a genuinely malformed
+  *artifact* file (`ParseError`) and stays silent for `NotArtifactFile`.
+  `rivet validate` still re-scans and surfaces the malformed case as a hard
+  `artifact-parse-error` Error, so nothing is lost. Regression test.
+
 - **REQ-115 / REQ-116 / REQ-118 — Zola export links survive a sub-directory
   deploy.** Artifact cross-links, document `[[ID]]` wiki-links, and the
   `rivet_artifact` shortcode card all emitted absolute `/<prefix>/artifacts/…`

artifacts/requirements.yaml

@@ -3787,3 +3787,85 @@ artifacts:
     links:
       - type: traces-to
         target: REQ-115
+
+  - id: REQ-139
+    type: requirement
+    title: "Directory import: only warn on malformed artifact files, stay silent for legitimate non-artifact YAML"
+    status: implemented
+    description: |
+      Self-reported via GitHub issue #353 part 4 (dogfooding meld). The
+      generic-YAML directory import warned `[WARN] skipping <file>: …` for
+      EVERY file it declined to load — including legitimate non-artifact YAML
+      that lives under the artifacts source path (bindings.yaml,
+      feature-model.yaml, variants/*.yaml). On a real project that is a WARN
+      line on every single command, burying the signal (loud-fail register:
+      noise that cries wolf is as bad as silence).
+
+      The REQ-062 `SkipKind` classification already distinguishes a malformed
+      artifact file (`ParseError`) from expected non-artifact YAML
+      (`NotArtifactFile`), but the load-path WARN ignored it. Gate the WARN
+      on `classify_skip(...) == ParseError`: a real problem still warns (and
+      `validate` still surfaces it as a hard `artifact-parse-error`); the
+      expected case is silent.
+
+      Acceptance:
+        - `rivet list` on a project whose artifacts/ dir contains
+          bindings.yaml / variants/*.yaml prints no `skipping …` WARN lines.
+        - A genuinely malformed artifact file (e.g. `id:`+`type:` at top
+          level, or `artifacts:` plus an unknown sibling key) still prints a
+          `skipping …` WARN at load and a `artifact-parse-error` ERROR under
+          `rivet validate`.
+    tags: [dx, agent-ux, signal-to-noise, loud-fail, user-reported, issue-353]
+    fields:
+      priority: should
+      category: functional
+      baseline: v0.15.0-track
+    links:
+      - type: traces-to
+        target: REQ-062
+
+  - id: REQ-140
+    type: requirement
+    title: "Generic-YAML load is inconsistent across commands: validate loads a file that list/get/export silently drop"
+    status: draft
+    description: |
+      Surfaced while verifying issue #353 against current `main` (the silent
+      file-skip of #353 part 1 is already fixed by REQ-062). A file with an
+      `artifacts:` list AND an unknown sibling top-level key (the reporter's
+      `traceability.yaml` with `loss-coverage:`) is handled inconsistently:
+
+        * `rivet list` / `get` / `export` build the store via the strict
+          `load_artifacts` adapter (serde `deny_unknown_fields`) and DROP the
+          whole file — its artifacts are invisible to these commands.
+        * `rivet validate` builds its store via the lenient
+          `yaml_hir::extract_schema_driven` path, which tolerates the unknown
+          key and LOADS the artifacts — then ALSO reports the same file as
+          `artifact-parse-error` "failed to parse".
+
+      So the same project yields different artifact sets depending on the
+      command, and `validate` simultaneously validates an artifact (REQ-2)
+      and declares its file unparseable — a contradictory signal. Verified
+      empirically: `rivet list` shows only REQ-1; `rivet validate` shows
+      REQ-2 as a loaded (mis-typed) artifact plus the parse-error.
+
+      This is a design decision (which parser is canonical, and is the
+      tolerant behaviour intended?) — flagging for the maintainer rather than
+      unilaterally picking. The fix should make every command agree, on the
+      loud-fail side: a file that can't be cleanly parsed as artifacts should
+      be treated identically everywhere (either all load-and-warn, or all
+      drop-and-error), never load in one command and vanish in another.
+
+      Acceptance:
+        - For a fixture file with `artifacts:` + an unknown sibling key,
+          `rivet list --format json` and the set `rivet validate` type-checks
+          contain the SAME artifact IDs.
+        - `rivet validate` does not both load an artifact and report its
+          source file as failed-to-parse.
+    tags: [bug, consistency, loud-fail, parser, user-reported, issue-353, needs-maintainer-decision]
+    fields:
+      priority: should
+      category: functional
+      baseline: v0.15.0-track
+    links:
+      - type: traces-to
+        target: REQ-062

rivet-core/src/formats/generic.rs

@@ -223,7 +223,24 @@ fn import_generic_directory(dir: &Path) -> Result<Vec<Artifact>, Error> {
         {
             match import_generic_file(&path) {
                 Ok(arts) => artifacts.extend(arts),
-                Err(e) => log::warn!("skipping {}: {}", path.display(), e),
+                Err(e) => {
+                    // REQ-062 / #353: only a *malformed artifact file*
+                    // (SkipKind::ParseError) is a real problem worth a
+                    // per-command WARN. Legitimate non-artifact YAML living
+                    // under the source path — bindings.yaml,
+                    // feature-model.yaml, variants/*.yaml — classifies as
+                    // NotArtifactFile and is skipped silently; warning on it
+                    // every single command is pure noise that buries the
+                    // signal. `rivet validate` still re-scans and surfaces the
+                    // ParseError case as a hard `artifact-parse-error`
+                    // diagnostic, so suppressing the noisy case loses nothing.
+                    let is_parse_error = std::fs::read_to_string(&path)
+                        .map(|c| classify_skip(&c) == SkipKind::ParseError)
+                        .unwrap_or(true); // unreadable -> surface it
+                    if is_parse_error {
+                        log::warn!("skipping {}: {}", path.display(), e);
+                    }
+                }
             }
         } else if path.is_dir() {
             artifacts.extend(import_generic_directory(&path)?);
@@ -504,4 +521,54 @@ Artifacts:
             "a well-formed `artifacts:` file must produce no skip entry"
         );
     }
+
+    /// #353: a directory import must load the well-formed artifact files and
+    /// skip BOTH a legitimate non-artifact file (`bindings.yaml`,
+    /// NotArtifactFile — skipped silently, no WARN) and a malformed artifact
+    /// file (ParseError — skipped here, but surfaced by `validate`'s re-scan)
+    /// without erroring the whole load. This pins the load-path classification
+    /// that gates the per-command WARN: only ParseError warrants a WARN; the
+    /// NotArtifactFile case is expected and silent.
+    ///
+    /// rivet: verifies REQ-062
+    #[test]
+    fn import_directory_loads_valid_and_skips_both_skip_kinds() {
+        let dir = tempfile::tempdir().unwrap();
+        std::fs::write(
+            dir.path().join("requirements.yaml"),
+            "artifacts:\n  - id: REQ-1\n    type: requirement\n    title: Valid\n",
+        )
+        .unwrap();
+        // NotArtifactFile — must be classified as such (silent skip).
+        let bindings = dir.path().join("bindings.yaml");
+        std::fs::write(&bindings, "bindings:\n  - core\n").unwrap();
+        // ParseError — `artifacts:` plus an unknown sibling top-level key.
+        let trace = dir.path().join("traceability.yaml");
+        std::fs::write(
+            &trace,
+            "loss-coverage:\n  - loss: L-1\nartifacts:\n  - id: REQ-2\n    type: requirement\n    title: Dropped\n",
+        )
+        .unwrap();
+
+        let arts = import_generic_directory(dir.path()).expect("directory import must not error");
+        let ids: Vec<&str> = arts.iter().map(|a| a.id.as_str()).collect();
+        assert_eq!(
+            ids,
+            vec!["REQ-1"],
+            "only the well-formed artifact file loads; both skipped files are excluded"
+        );
+
+        // The two skipped files classify into the two distinct kinds — the
+        // load-path WARN suppression keys off exactly this distinction.
+        assert_eq!(
+            classify_skip(&std::fs::read_to_string(&bindings).unwrap()),
+            SkipKind::NotArtifactFile,
+            "bindings.yaml is expected non-artifact YAML — silent skip"
+        );
+        assert_eq!(
+            classify_skip(&std::fs::read_to_string(&trace).unwrap()),
+            SkipKind::ParseError,
+            "an `artifacts:` file with an unknown sibling key is a malformed artifact file"
+        );
+    }
 }