fix(dump,rust): type-faithful preserve edge cases + rust rpath, from adversarial review

Follow-up to abbe811, addressing every finding of a three-lens
adversarial review plus the broken rust cross-language backend.

Rust backend (never actually ran)
- examples/tryparse links libl4yaml.so and libleanshared.so but cargo
  baked no rpath, so every suiterunner invocation died with exit 127
  (loader error) and scored as a parse rejection — masked by tee until
  the pipefail fix. l4yaml-sys/build.rs now exports both library dirs
  over its links="l4yaml" metadata channel and a new l4yaml/build.rs
  turns them into -Wl,-rpath args (the cargo mirror of tryparse_c's
  CMake BUILD_RPATH). Verified: binary runs with no LD_LIBRARY_PATH;
  all four backends now agree exactly on all five presets
  (unlimited 783/86/151; others 774/95/151).

ScalarPref.preserve edge cases (review blockers/minors)
- Plain negative numbers re-quoted: isPlainSafe rejects every leading
  indicator, so preserve turned plain -1 into "-1" — which the new
  style-aware safe_load reads back as a STRING. New isPlainSafePreserve
  admits leading '-' followed by a non-space (ns-plain-first [126]),
  used only in the preserve arm; existing prefs untouched.
- Literal/folded content without a honored newline fell into the plain
  path (`>- 42` re-emitted as plain 42, string→int). Block styles now
  fall back to double quotes: block scalars always resolve to strings.
- Single-quoted content with CR/C0/C1 controls was emitted raw (CR
  reparses as folding — silent corruption). New singleQuotedRepresentable
  (printable+tab per c-printable) gates the style; else escaped
  double quotes.
- 17 new #guard tests pin the preserve semantics (Tests/Guards/Dump.lean).

Python compat (review blockers/minors)
- Mapping KEYS lost their types: _dump_mapping quoted every key via
  _quote_scalar(str(key)), so {42: ...} came back {'42': ...}. Keys now
  serialize through _python_to_yaml (non-scalar keys raise TypeError,
  as pyyaml does).
- _fidelity_config now validates known DumpConfig fields (types, enums,
  Nat-ness of indent/lineWidth) and wraps malformed config YAML in
  ConfigError: the Lean config reader silently falls back to ALL
  defaults on any bad field, which would quietly discard the fidelity
  settings over a cosmetic typo.
- safe_dump_all newline-terminates chunks so '---' no longer glues onto
  the previous document (stream now reparses to the same documents).
- _find_library: directory-valued L4YAML_LIB now tries platform library
  names (darwin .dylib included); stale pre-0.5.0 libraries produce an
  OSError with a rebuild hint instead of an opaque AttributeError.
- 10 new pytest regressions (negative ints/floats, non-string keys,
  config validation, dump_all reparse).

Tests and CI
- Dropped test_tab_as_separation_accepted: it codified acceptance of
  "key:\n\t value", a leniency no yaml-test-suite case adjudicates
  (Y79Y pins tab-before-indicator as fail, tab-before-scalar same-line
  as valid; the next-line form is spec-questionable and PyYAML rejects
  it). The rejection test keeps its probed input; the comment now
  states the leniency without asserting it.
- Workflow venv step: PYTHONPATH/PYTHONNOUSERSITE now exported locally
  before the in-step pip/pytest checks (GITHUB_ENV only affects later
  steps), and venv creation failure gets an actionable error.
- TryDump.lean doc lists the "preserve" scalarStyle.

Verified: full lake build green (all guards), dumproundtrip 117/117
(default config unchanged), 206/206 across both pytest suites under the
hermetic CI environment, and a follow-up confirmation review reported
10/10 fix areas confirmed; its one new finding (negative indent
bypassing validation into the same silent-fallback) is fixed and
tested here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: NicolasRouquette

.github/workflows/test-coverage.yml

@@ -48,13 +48,10 @@ jobs:
           # ambient PATH says.
           PYBASE=/usr/bin/python3.12
           if [ ! -x "$PYBASE" ]; then PYBASE=/usr/bin/python3; fi
-          "$PYBASE" -m venv "$RUNNER_TEMP/ci-venv"
-          VPY="$RUNNER_TEMP/ci-venv/bin/python3"
-          "$VPY" -m pip install --quiet --disable-pip-version-check --upgrade pip
-          "$VPY" -m pip install --quiet --disable-pip-version-check pytest
-          "$VPY" --version
-          "$VPY" -m pytest --version
-          echo "$RUNNER_TEMP/ci-venv/bin" >> "$GITHUB_PATH"
+          "$PYBASE" -m venv "$RUNNER_TEMP/ci-venv" || {
+            echo "::error::venv creation with $PYBASE failed — the runner needs a python with the venv module (Debian/Ubuntu: apt install python3-venv)"
+            exit 1
+          }
           # A venv does NOT isolate against PYTHONPATH: its entries come
           # before both the stdlib and the venv's site-packages on
           # sys.path. On this box, ROS setup scripts export
@@ -63,10 +60,20 @@ jobs:
           # of the old `-p no:launch_ros` flags). GITHUB_ENV cannot
           # unset, and PYTHONPATH="" would put the cwd on sys.path, so
           # point it at an empty directory; PYTHONNOUSERSITE blocks
-          # ~/.local site-packages the same way.
+          # ~/.local site-packages the same way. Export locally too:
+          # the GITHUB_ENV write only affects LATER steps, and the
+          # pip/pytest checks below must not load foreign site-packages.
           mkdir -p "$RUNNER_TEMP/ci-venv/empty-pythonpath"
-          echo "PYTHONPATH=$RUNNER_TEMP/ci-venv/empty-pythonpath" >> "$GITHUB_ENV"
+          export PYTHONPATH="$RUNNER_TEMP/ci-venv/empty-pythonpath"
+          export PYTHONNOUSERSITE=1
+          echo "PYTHONPATH=$PYTHONPATH" >> "$GITHUB_ENV"
           echo "PYTHONNOUSERSITE=1" >> "$GITHUB_ENV"
+          VPY="$RUNNER_TEMP/ci-venv/bin/python3"
+          "$VPY" -m pip install --quiet --disable-pip-version-check --upgrade pip
+          "$VPY" -m pip install --quiet --disable-pip-version-check pytest
+          "$VPY" --version
+          "$VPY" -m pytest --version
+          echo "$RUNNER_TEMP/ci-venv/bin" >> "$GITHUB_PATH"
 
       - name: Install weasyprint for PDF generation
         id: weasyprint

L4YAML/Output/Dump.lean

@@ -251,6 +251,30 @@ def isPlainSafe (s : String) (allowReserved : Bool := false) : Bool :=
 def hasNewlines (s : String) : Bool :=
   s.any (· == '\n')
 
+/-- `ns-plain-first` refinement used only by `ScalarPref.preserve`:
+    `isPlainSafe` rejects every leading indicator, but YAML [126]
+    permits a leading `-` when followed by a non-space character
+    (`-1`, `-.inf`). Without this, a parsed plain `-1` would be
+    re-emitted quoted — changing its core-schema type to string.
+    Kept separate from `isPlainSafe` so the existing prefs (and the
+    output calibrated to them) are untouched. -/
+def isPlainSafePreserve (s : String) (allowReserved : Bool := false) : Bool :=
+  isPlainSafe s allowReserved ||
+  (match s.toList with
+   | '-' :: c :: rest =>
+     c != ' ' && c != '\t' && isPlainSafe (String.ofList (c :: rest)) allowReserved
+   | _ => false)
+
+/-- Whether content can be carried VERBATIM inside single quotes:
+    printable characters only (plus tab; C0, DEL, and C1 excluded per
+    c-printable §5.1). A raw CR reparses as line folding (silent
+    corruption) and other controls are invalid YAML there;
+    double-quoting escapes them instead. -/
+def singleQuotedRepresentable (s : String) : Bool :=
+  s.all (fun c => c == '\t' ||
+    (0x20 ≤ c.toNat && c.toNat != 0x7F &&
+     !(0x80 ≤ c.toNat && c.toNat ≤ 0x9F)))
+
 /-- Check if string content is unsafe as a plain scalar in flow context.
     Flow context forbids additional characters beyond what `isPlainSafe` checks:
     - Any `:` (not just `: `), since `:` followed by `,`, `}`, `]` is a mapping indicator
@@ -298,16 +322,21 @@ def chooseScalarStyle (s : Scalar) (cfg : DumpConfig)
       match s.style with
       | .doubleQuoted => .doubleQuoted
       | .singleQuoted =>
-        -- Single-quoted cannot represent newlines
-        if hasNewlines s.content then .doubleQuoted else .singleQuoted
-      | _ =>
-        -- Plain (or a block style not already honored by the outer
-        -- literal/folded case): plain when safe, else double-quoted.
-        if !s.content.isEmpty && !hasNewlines s.content &&
-            isPlainSafe s.content cfg.allowReservedPlain &&
+        -- Newlines/controls cannot be carried verbatim in single quotes
+        if singleQuotedRepresentable s.content then .singleQuoted
+        else .doubleQuoted
+      | .plain =>
+        if !hasNewlines s.content &&
+            isPlainSafePreserve s.content cfg.allowReservedPlain &&
             (ctx == .block || !isFlowUnsafe s.content) then
           .plain
         else .doubleQuoted
+      | _ =>
+        -- A literal/folded style NOT honored by the outer newline+block
+        -- case (no newline in content, or flow context): block scalars
+        -- always resolve to strings (§10.3.2), so quote — re-emitting
+        -- plain could flip the type (`>- 42` must not become `42`).
+        .doubleQuoted
 
 /-- Resolve collection style from node annotation, config, and dump context.
     When context is flow, block collections are forced to flow (YAML §8.1

Tests/Guards/Dump.lean

@@ -186,4 +186,52 @@ private def docADir : YamlDocument :=
 #guard dump (.sequence .block #[.plainScalar "a", .plainScalar "b"])
   { compactSequenceMap := true } == "- a\n- b"
 
+/-! ### scalarStyle preserve: honor node style, keep core-schema type
+
+The type-fidelity config used by the Python binding's `safe_dump`
+(`{ scalarStyle := .preserve, allowReservedPlain := true }`): each
+scalar re-emits in its own style, so quoting — and therefore the type
+a core-schema consumer resolves — survives a load→dump round trip. -/
+
+-- Plain stays plain, including leading '-' (ns-plain-first
+-- refinement `isPlainSafePreserve`; plain `-1` must not re-quote
+-- into a string) and reserved words under allowReservedPlain.
+#guard dump (.plainScalar "-1")
+  { scalarStyle := .preserve, allowReservedPlain := true } == "-1"
+#guard dump (.plainScalar "-3.14")
+  { scalarStyle := .preserve, allowReservedPlain := true } == "-3.14"
+#guard dump (.plainScalar "-.inf")
+  { scalarStyle := .preserve, allowReservedPlain := true } == "-.inf"
+#guard dump (.plainScalar "true")
+  { scalarStyle := .preserve, allowReservedPlain := true } == "true"
+#guard dump (.plainScalar "42") { scalarStyle := .preserve } == "42"
+-- Without allowReservedPlain, reserved words still quote.
+#guard dump (.plainScalar "true") { scalarStyle := .preserve } == "\"true\""
+-- Plain content that is not plain-safe still quotes ('- x' is a
+-- sequence-entry lookalike; empty needs quotes).
+#guard dump (.plainScalar "- x") { scalarStyle := .preserve } == "\"- x\""
+#guard dump (.plainScalar "key: value")
+  { scalarStyle := .preserve } == "\"key: value\""
+#guard dump (.plainScalar "") { scalarStyle := .preserve } == "\"\""
+-- Quoted stays quoted: '42' must not re-emit plain (type would flip
+-- string→int under core-schema resolution).
+#guard dump (.quotedScalar "42" .singleQuoted)
+  { scalarStyle := .preserve } == "'42'"
+#guard dump (.quotedScalar "true" .singleQuoted)
+  { scalarStyle := .preserve, allowReservedPlain := true } == "'true'"
+#guard dump (.quotedScalar "42" .doubleQuoted)
+  { scalarStyle := .preserve } == "\"42\""
+-- Block styles not honored by the newline case quote to stay strings
+-- (`>- 42` must not become plain 42).
+#guard dump (.scalar ⟨"42", .folded, none, none, none⟩)
+  { scalarStyle := .preserve, allowReservedPlain := true } == "\"42\""
+#guard dump (.scalar ⟨"true", .literal, none, none, none⟩)
+  { scalarStyle := .preserve, allowReservedPlain := true } == "\"true\""
+-- Single-quoted content that single quotes cannot carry verbatim
+-- (CR/C0 controls, newlines) falls back to escaped double quotes.
+#guard dump (.scalar ⟨"a\rb", .singleQuoted, none, none, none⟩)
+  { scalarStyle := .preserve } == "\"a\\rb\""
+#guard dump (.scalar ⟨"line1\nline2", .singleQuoted, none, none, none⟩)
+  { scalarStyle := .preserve } == "\"line1\\nline2\""
+
 end L4YAML.Dump

Tests/TryDump.lean

@@ -28,7 +28,7 @@ All fields are optional (defaults match `DumpConfig {}`):
 ```
 
 `defaultStyle`: `"block"` | `"flow"` | `"auto"`
-`scalarStyle`: `"plain"` | `"doubleQuoted"` | `"singleQuoted"` | `"auto"`
+`scalarStyle`: `"plain"` | `"doubleQuoted"` | `"singleQuoted"` | `"auto"` | `"preserve"`
 
 ## Exit codes
 

Tests/test_python_ffi.py

@@ -421,19 +421,15 @@ def test_unclosed_flow_mapping(self) -> None:
             l4yaml.load("{unclosed")
 
     def test_tab_in_indentation(self) -> None:
-        # "key:\n\t value" is NOT an error: the line's indentation is
-        # zero spaces, and the tab is separation space before the
-        # plain scalar (yaml-test-suite tab semantics). A tab used as
-        # block indentation before an indicator IS an error:
+        # Tab used as block indentation before an indicator is an
+        # error (cf. yaml-test-suite Y79Y: `-\t-` is fail:true).
+        # NB the parser currently ACCEPTS a tab before a plain scalar
+        # on a value line ("key:\n\t value") — a leniency no suite
+        # case adjudicates and PyYAML rejects — so that input is
+        # deliberately not asserted either way here.
         with pytest.raises(l4yaml.ParseError):
             l4yaml.load("a:\n\t- x")
 
-    def test_tab_as_separation_accepted(self) -> None:
-        # Companion to test_tab_in_indentation: tab as separation
-        # space before a plain scalar is legal.
-        v = l4yaml.load("key:\n\t value")
-        assert v.as_dict()["key"].as_str() == "value"
-
     def test_error_message_nonempty(self) -> None:
         with pytest.raises(l4yaml.ParseError) as exc_info:
             l4yaml.load("[unclosed")

python/l4yaml/_ffi.py

@@ -29,14 +29,19 @@
 
 def _find_library() -> Path:
     """Locate libl4yaml.so by searching several candidate paths."""
-    # 1. Explicit environment variable: the .so itself, or a directory
-    #    containing it.
+    # 1. Explicit environment variable: the library itself, or a
+    #    directory containing it (platform-appropriate name).
+    lib_names: list[str] = ["libl4yaml.so"]
+    if sys.platform == "darwin":
+        lib_names.append("libl4yaml.dylib")
     env_path: str | None = os.environ.get("L4YAML_LIB")
     if env_path:
         p = Path(env_path)
         if p.is_dir():
-            p = p / "libl4yaml.so"
-        if p.is_file():
+            for name in lib_names:
+                if (p / name).is_file():
+                    return p / name
+        elif p.is_file():
             return p
 
     # 2. Relative to this package: pkg_dir is <repo>/python/l4yaml, so
@@ -61,9 +66,9 @@ def _find_library() -> Path:
             return candidate
 
     raise OSError(
-        "Cannot find libl4yaml.so. Set L4YAML_LIB environment "
-        "variable to the full path, or place the library next to this "
-        "package."
+        "Cannot find libl4yaml.so. Set the L4YAML_LIB environment "
+        "variable to the library file or its directory, or place the "
+        "library next to this package."
     )
 
 
@@ -148,8 +153,16 @@ def _load_lib() -> ctypes.CDLL:
     lib.l4yaml_value_kind.argtypes = [c_void_p]
     lib.l4yaml_value_kind.restype = c_uint8
 
-    lib.l4yaml_value_scalar_style.argtypes = [c_void_p]
-    lib.l4yaml_value_scalar_style.restype = c_uint8
+    try:
+        lib.l4yaml_value_scalar_style.argtypes = [c_void_p]
+        lib.l4yaml_value_scalar_style.restype = c_uint8
+    except AttributeError as exc:
+        # OSError (not AttributeError) so callers' library-missing
+        # handling — e.g. the test suites' needs_lib skip — applies.
+        raise OSError(
+            "libl4yaml.so predates the scalar-style API (v0.5.0) — "
+            "rebuild it: cmake -B ffi/out -S ffi && cmake --build ffi/out"
+        ) from exc
 
     lib.l4yaml_value_string.argtypes = [c_void_p]
     lib.l4yaml_value_string.restype = c_char_p

python/l4yaml/compat.py

@@ -352,6 +352,10 @@ def safe_dump_all(
     for doc in documents:
         chunk: str | None = safe_dump(doc, config=config)
         if chunk is not None:
+            # The dumper emits no trailing newline; without one the
+            # '---' separator would glue onto the previous document.
+            if not chunk.endswith("\n"):
+                chunk += "\n"
             parts.append(chunk)
     result: str = "---\n".join(parts)
     if stream is not None:
@@ -363,6 +367,22 @@ def safe_dump_all(
 # ── Dump config with type fidelity ───────────────────────────────────
 
 
+# DumpConfig fields (L4YAML/Output/Dump.lean) and their value shapes,
+# used to validate user configs eagerly: the Lean config reader falls
+# back to ALL defaults when any field fails to parse, which would
+# silently drop the fidelity settings below over a cosmetic typo.
+_DUMP_CONFIG_SPEC: dict[str, Any] = {
+    "indent": int,
+    "lineWidth": int,
+    "sortKeys": bool,
+    "allowReservedPlain": bool,
+    "omitEmpty": bool,
+    "compactSequenceMap": bool,
+    "defaultStyle": ("block", "flow", "auto"),
+    "scalarStyle": ("plain", "doubleQuoted", "singleQuoted", "auto", "preserve"),
+}
+
+
 def _fidelity_config(user_cfg: str | None) -> str:
     """Merge type-fidelity defaults into a user dump config.
 
@@ -373,12 +393,42 @@ def _fidelity_config(user_cfg: str | None) -> str:
     ``true``/``null`` (so they stay a bool/null). Explicit user
     settings for either key win. The merged config is emitted as
     JSON, which the config parser reads as flow-style YAML.
+
+    Raises:
+        ConfigError: If the config is not valid YAML, not a mapping,
+            or a known field has a value the dump config would reject.
     """
-    merged: Any = safe_load(user_cfg) if user_cfg is not None else {}
-    if merged is None:
-        merged = {}
+    if user_cfg is None:
+        merged: Any = {}
+    else:
+        try:
+            merged = safe_load(user_cfg)
+        except L4YAMLError as exc:
+            raise ConfigError(f"invalid dump config YAML: {exc}") from exc
+        if merged is None:
+            merged = {}
     if not isinstance(merged, dict):
         raise ConfigError(f"dump config must be a mapping, got: {merged!r}")
+    for key, spec in _DUMP_CONFIG_SPEC.items():
+        if key not in merged:
+            continue
+        val = merged[key]
+        if isinstance(spec, tuple):
+            ok = val in spec
+            expected = " | ".join(spec)
+        elif spec is int:
+            # The Lean fields are Nat: bools and negatives would fail
+            # its reader and silently drop the whole config.
+            ok = isinstance(val, int) and not isinstance(val, bool) and val >= 0
+            expected = "non-negative int"
+        else:
+            ok = isinstance(val, spec)
+            expected = spec.__name__
+        if not ok:
+            raise ConfigError(
+                f"dump config field {key!r}: invalid value {val!r} "
+                f"(expected {expected})"
+            )
     merged.setdefault("scalarStyle", "preserve")
     merged.setdefault("allowReservedPlain", True)
     return json.dumps(merged)
@@ -470,7 +520,11 @@ def _dump_mapping(mapping: dict[Any, Any], indent: int) -> str:
     prefix: str = "  " * indent
     lines: list[str] = []
     for key, val in mapping.items():
-        key_str: str = _quote_scalar(str(key))
+        # Serialize keys with type fidelity: _quote_scalar(str(key))
+        # would quote non-string keys (42 -> '42'), pinning them to str
+        # under style-aware loading. Non-scalar keys raise TypeError
+        # (as pyyaml's safe_dump rejects them).
+        key_str: str = _python_to_yaml(key)
         if isinstance(val, (dict, list)) and val:
             child: str = _python_to_yaml(val, indent + 1)
             lines.append(f"{prefix}{key_str}:\n{child}")