Repository: tweag/nickel
Pull request #2012 (2024-07-26 09:02:35)

Track benchmarks in CI
This is an initial attempt to track benchmarks in CI.

It runs (just a few) benchmarks on every push to master, and on each pr it also runs a few benchmarks and compares them to master. It uses bencher.dev to track benchmark results over time.

This probably doesn't work yet. I've been trying to debug why the PR benchmark result uploads don't work; it turns out the workflow file needs to be in the default branch before it will run...


Patch:
diff --git a/.github/workflows/benchmark-master.yaml b/.github/workflows/benchmark-master.yaml
new file mode 100644
index 00000000..7e317448
--- /dev/null
+++ b/.github/workflows/benchmark-master.yaml
@@ -0,0 +1,29 @@
+name: benchmark master
+on:
+  push:
+    branches: master
+jobs:
+  benchmark-master:
+    name: benchmark master
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - uses: cachix/install-nix-action@V27
+        name: "Installing Nix"
+        with:
+          extra_nix_config: |
+            experimental-features = nix-command flakes
+            accept-flake-config = true
+          nix_path: "nixpkgs=channel:nixos-unstable"
+      - uses: bencherdev/bencher@main
+      - name: Track benchmarks on master
+        run: |
+          bencher run \
+          --project nickel \
+          --token '${{ secrets.BENCHER_API_TOKEN }}' \
+          --branch master \
+          --testbed ubuntu-latest \
+          --adapter rust_criterion \
+          nix develop --command cargo bench --package nickel-lang-core --bench numeric
diff --git a/.github/workflows/benchmark-pr.yaml b/.github/workflows/benchmark-pr.yaml
new file mode 100644
index 00000000..73ac2a2a
--- /dev/null
+++ b/.github/workflows/benchmark-pr.yaml
@@ -0,0 +1,39 @@
+# This is the part of the benchmark runner that runs on pull requests.
+# It's separate from the master runner for two reasons:
+# - to track it differently on bencher.dev, because we only want to use master as
+#   the historical baseline for comparison
+# - because it might want to run on PRs from forks, and those don't get access to the
+#   bencher API key.
+#
+# See the [bencher docs] for more details.
+#
+# [bencher docs]:  https://bencher.dev/docs/how-to/github-actions/#benchmark-fork-pr-and-upload-from-default-branch
+
+name: benchmark pull requests
+on:
+  pull_request:
+    types: [opened, reopened, edited, synchronize]
+
+jobs:
+  benchmark-pr:
+    name: benchmark pr
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+      - uses: cachix/install-nix-action@V27
+        name: "Installing Nix"
+        with:
+          extra_nix_config: |
+            experimental-features = nix-command flakes
+            accept-flake-config = true
+          nix_path: "nixpkgs=channel:nixos-unstable"
+      - name: run benchmarks
+        run: |
+          nix develop --command cargo bench --package nickel-lang-core --bench numeric > criterion-output.txt
+      - name: upload results
+        uses: actions/upload-artifact@v4
+        with:
+          name: criterion-output
+          path: criterion-output.txt
diff --git a/.github/workflows/release-artifacts.yaml b/.github/workflows/release-artifacts.yaml
index d9cee164..b9eb3e7d 100644
--- a/.github/workflows/release-artifacts.yaml
+++ b/.github/workflows/release-artifacts.yaml
@@ -1,4 +1,4 @@
-name: Upload release artifacts
+name: Test Docker manifest
 on:
   release:
     types: [published]
diff --git a/.github/workflows/run-benchmark.yaml b/.github/workflows/run-benchmark.yaml
deleted file mode 100644
index b1a0ee3a..00000000
--- a/.github/workflows/run-benchmark.yaml
+++ /dev/null
@@ -1,17 +0,0 @@
-name: benchmark pull requests
-on:
-  pull_request:
-    types: [labeled]
-jobs:
-  runBenchmark:
-    if: ${{ github.event.label.name == 'run-benchmark' && github.event_name == 'pull_request' }}
-    name: run benchmark
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          fetch-depth: 0
-      - uses: jasonwilliams/criterion-compare-action@move_to_actions
-        with:
-          branchName: ${{ github.base_ref }}
-          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/upload-pr-benchmarks.yaml b/.github/workflows/upload-pr-benchmarks.yaml
new file mode 100644
index 00000000..f8f7f3dc
--- /dev/null
+++ b/.github/workflows/upload-pr-benchmarks.yaml
@@ -0,0 +1,72 @@
+name: Track Benchmarks with Bencher
+
+on:
+  workflow_run:
+    workflows: [benchmark pull requests]
+    types: [completed]
+
+jobs:
+  track_fork_pr_branch:
+    name: track pr benchmarks
+    if: ${{ github.event.workflow_run.conclusion == 'success' }}
+    runs-on: ubuntu-latest
+    env:
+      BENCHMARK_RESULTS: criterion-output
+      PR_EVENT: event.json
+    steps:
+      - name: Download Benchmark Results
+        uses: actions/github-script@v6
+        with:
+          script: |
+            async function downloadArtifact(artifactName) {
+              let allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                run_id: context.payload.workflow_run.id,
+              });
+              let matchArtifact = allArtifacts.data.artifacts.filter((artifact) => {
+                return artifact.name == artifactName
+              })[0];
+              if (!matchArtifact) {
+                core.setFailed(`Failed to find artifact: ${artifactName}`);
+              }
+              let download = await github.rest.actions.downloadArtifact({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                artifact_id: matchArtifact.id,
+                archive_format: 'zip',
+              });
+              let fs = require('fs');
+              fs.writeFileSync(`${process.env.GITHUB_WORKSPACE}/${artifactName}.zip`, Buffer.from(download.data));
+            }
+            await downloadArtifact(process.env.BENCHMARK_RESULTS);
+            await downloadArtifact(process.env.PR_EVENT);
+      - name: Unzip Benchmark Results
+        run: |
+          unzip $BENCHMARK_RESULTS.zip
+          unzip $PR_EVENT.zip
+      - name: Export PR Event Data
+        uses: actions/github-script@v6
+        with:
+          script: |
+            let fs = require('fs');
+            let prEvent = JSON.parse(fs.readFileSync(process.env.PR_EVENT, {encoding: 'utf8'}));
+            core.exportVariable("PR_HEAD", `${prEvent.number}/merge`);
+            core.exportVariable("PR_BASE", prEvent.pull_request.base.ref);
+            core.exportVariable("PR_BASE_SHA", prEvent.pull_request.base.sha);
+            core.exportVariable("PR_NUMBER", prEvent.number);
+      - uses: bencherdev/bencher@main
+      - name: Track Benchmarks with Bencher
+        run: |
+          bencher run \
+          --project nickel \
+          --token '${{ secrets.BENCHER_API_TOKEN }}' \
+          --branch '${{ env.PR_HEAD }}' \
+          --branch-start-point '${{ env.PR_BASE }}' \
+          --branch-start-point-hash '${{ env.PR_BASE_SHA }}' \
+          --testbed ubuntu-latest \
+          --adapter criterion_rust \
+          --err \
+          --github-actions '${{ secrets.GITHUB_TOKEN }}' \
+          --ci-number '${{ env.PR_NUMBER }}' \
+          --file "$BENCHMARK_RESULTS.txt"
diff --git a/core/benches/nixpkgs/lists.ncl b/core/benches/nixpkgs/lists.ncl
index d8d886cd..040ac812 100644
--- a/core/benches/nixpkgs/lists.ncl
+++ b/core/benches/nixpkgs/lists.ncl
@@ -525,9 +525,8 @@
             @ (toporest | { result : Array Dyn }).result
         } : { _ : Array Dyn },
 
-  #TODO: can it be statically typed?
   sort
-    | forall a. (a -> a -> Bool) -> Array a -> Array a
+    : forall a. (a -> a -> Bool) -> Array a -> Array a
     | doc m%"
       Sort a list based on a comparator function which compares two
      elements and returns true if the first argument is strictly below
diff --git a/core/src/eval/operation.rs b/core/src/eval/operation.rs
index b43f1ca1..6c8a054f 100644
--- a/core/src/eval/operation.rs
+++ b/core/src/eval/operation.rs
@@ -26,7 +26,6 @@ use crate::{
     mk_app, mk_fun, mk_record,
     parser::utils::parse_number_sci,
     position::TermPos,
-    pretty::PrettyPrintCap,
     serialize,
     serialize::ExportFormat,
     stdlib::internals,
@@ -40,6 +39,9 @@ use crate::{
     typecheck::eq::contract_eq,
 };
 
+#[cfg(feature = "metrics")]
+use crate::pretty::PrettyPrintCap;
+
 use malachite::{
     num::{
         arithmetic::traits::Pow,
@@ -1317,6 +1319,59 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                     pos_op_inh,
                 )))
             }
+            UnaryOp::ContractPostprocessResult => {
+                let (tag, arg) = match (*t).clone() {
+                    Term::EnumVariant { tag, arg, .. } => (tag, arg),
+                    _ => return mk_type_error!("[| 'Ok, 'Error _ |]"),
+                };
+
+                // We pop the second argument which isn't strict: we don't need to evaluate the
+                // label if there's no error.
+                let (label_closure, pos_label) = self.stack.pop_arg(&self.cache).unwrap();
+
+                match (tag.label(), arg) {
+                    ("Ok", value) => Ok(Closure { body: value, env }),
+                    ("Error", err_data) => {
+                        // In the error case, we first need to force the error data so that
+                        // primitive values (strings) can be extracted from it, attach the
+                        // corresponding data to the label, and then blame.
+                        //
+                        // To do so, we setup the stack to represent the evaluation context
+                        // `%contract/blame% (%label/with_error_data% (%force% [.]) label)` and
+                        // then continue with `err_data`.
+                        self.stack.push_op_cont(
+                            OperationCont::Op1(UnaryOp::Blame, arg_pos),
+                            self.call_stack.len(),
+                            pos_op_inh,
+                        );
+                        self.stack.push_op_cont(
+                            OperationCont::Op2First(
+                                BinaryOp::LabelWithErrorData,
+                                label_closure,
+                                pos_label,
+                            ),
+                            self.call_stack.len(),
+                            pos_op_inh,
+                        );
+                        self.stack.push_op_cont(
+                            OperationCont::Op1(
+                                UnaryOp::Force {
+                                    ignore_not_exported: false,
+                                },
+                                arg_pos,
+                            ),
+                            self.call_stack.len(),
+                            pos_op_inh,
+                        );
+
+                        Ok(Closure {
+                            body: err_data,
+                            env,
+                        })
+                    }
+                    _ => mk_type_error!("[| 'Ok, 'Error {..} |]'"),
+                }
+            }
             UnaryOp::NumberArcCos => Self::process_unary_number_operation(
                 RichTerm { term: t, pos },
                 arg_pos,
@@ -1682,26 +1737,35 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
             }
             BinaryOp::ContractApply | BinaryOp::ContractCheck => {
                 // The translation of a type might return any kind of contract, including e.g. a
-                // record or a custom contract. The result thus needs to be passed to
-                // `ContractApply` or `ContractCheck` again. In that case, we don't bother tracking
-                // the argument and updating the label: this will be done by the next call to
-                // `contract/apply(_as_custom)`.
+                // record or a custom contract. The result thus needs to be passed to `b_op` again.
+                // In that case, we don't bother tracking the argument and updating the label: this
+                // will be done by the next call to `b_op`.
                 if let Term::Type(typ) = &*t1 {
                     increment!(format!(
                         "primop:contract/apply:{}",
                         typ.pretty_print_cap(40)
                     ));
 
-                    return Ok(Closure {
-                        body: mk_term::op2(
+                    // We set the stack to represent the evaluation context `<b_op> [.] label` and
+                    // proceed to evaluate `<typ.contract()>`
+                    self.stack.push_op_cont(
+                        OperationCont::Op2First(
                             b_op,
-                            typ.contract()?,
-                            RichTerm {
-                                term: t2,
-                                pos: pos2,
+                            Closure {
+                                body: RichTerm {
+                                    term: t2,
+                                    pos: pos2,
+                                },
+                                env: env2,
                             },
-                        )
-                        .with_pos(pos1),
+                            fst_pos,
+                        ),
+                        self.call_stack.len(),
+                        pos_op_inh,
+                    );
+
+                    return Ok(Closure {
+                        body: typ.contract()?,
                         env: env1,
                     });
                 }
@@ -1719,81 +1783,189 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
                         increment!(format!("contract:originates_from_field {field}"));
                     }
 
-                    // Track the contract argument for better error reporting, and push back the
-                    // label on the stack, so that it becomes the first argument of the contract.
-                    let idx = self.stack.track_arg(&mut self.cache).ok_or_else(|| {
-                        EvalError::NotEnoughArgs(3, String::from("contract/apply"), pos_op)
-                    })?;
-
+                    // Pop the contract argument to track its cache index in the label for better
+                    // error reporting, and because we might add post-processing steps on the stack
+                    // which need to sit underneath the value and the label (they will be run after
+                    // the contract application is evaluated). We'll just push the value and the
+                    // label back on the stack at the end.
+                    let (idx, stack_value_pos) =
+                        self.stack.pop_arg_as_idx(&mut self.cache).ok_or_else(|| {
+                            EvalError::NotEnoughArgs(3, String::from("contract/apply"), pos_op)
+                        })?;
+
+                    // We update the label and convert it back to a term form that can be cheaply cloned
                     label.arg_pos = self.cache.get_then(idx.clone(), |c| c.body.pos);
-                    label.arg_idx = Some(idx);
-
-                    // We push the label back on the stack and we properly convert the
-                    // contract to a naked function of a label and a value, which will have the
-                    // stack in the same state as if it had been applied normally to both
-                    // arguments.
-                    self.stack.push_arg(
-                        Closure::atomic_closure(RichTerm::new(
-                            Term::Lbl(label.clone()),
-                            pos2.into_inherited(),
-                        )),
-                        pos2.into_inherited(),
-                    );
+                    label.arg_idx = Some(idx.clone());
+                    let new_label = RichTerm::new(Term::Lbl(label), pos2);
 
-                    match &*t1 {
+                    // If we're evaluating a plain contract application but we are applying
+                    // something with the signature of a custom contract, we need to setup some
+                    // post-processing.
+                    //
+                    // We prepare the stack so that `contract/postprocess_result` will be
+                    // applied afteward. This primop converts the result of a custom contract
+                    // `'Ok value` or `'Error err_data` to either `value` or a proper contract
+                    // error with `err_data` included in the label.
+                    //
+                    // That is, prepare the stack to represent the evaluation context
+                    // `%contract/postprocess_result% [.] label`
+                    if matches!(
+                        (&*t1, &b_op),
+                        (
+                            Term::CustomContract(_) | Term::Record(_),
+                            BinaryOp::ContractApply
+                        )
+                    ) {
+                        self.stack
+                            .push_arg(Closure::atomic_closure(new_label.clone()), pos_op_inh);
+
+                        self.stack.push_op_cont(
+                            OperationCont::Op1(
+                                UnaryOp::ContractPostprocessResult,
+                                pos1.into_inherited(),
+                            ),
+                            self.call_stack.len(),
+                            pos_op_inh,
+                        );
+                    }
+
+                    // Now that we've updated the label, we push the checked value and the new
+                    // label back on the stack, so that they become the two arguments of the
+                    // contract (transformed to something that can be applied directly). That is,
+                    // we prepare the stack to represent the evaluation context `[.] label value`
+                    // and proceed with the evaluation of `functoid`.
+                    self.stack.push_tracked_arg(idx, stack_value_pos);
+                    self.stack
+                        .push_arg(Closure::atomic_closure(new_label), pos2.into_inherited());
+
+                    // We convert the contract (which can be a custom contract, a record, a naked
+                    // function, etc.) to a form that can be applied to a label and a value.
+                    let functoid = match &*t1 {
                         Term::Fun(..) | Term::Match { .. } => {
                             let as_naked = RichTerm {
                                 term: t1,
                                 pos: pos1,
                             };
 
-                            let body = if let BinaryOp::ContractApply = b_op {
-                                as_naked
+                            if let BinaryOp::ContractApply = b_op {
+                                Closure {
+                                    body: as_naked,
+                                    env: env1,
+                                }
                             } else {
-                                mk_app!(internals::naked_to_custom(), as_naked).with_pos(pos1)
-                            };
+                                // Prepare the stack to represent the evaluation context `[.]
+                                // as_naked` and proceed with `$naked_to_custom`
+                                self.stack.push_arg(
+                                    Closure {
+                                        body: as_naked,
+                                        env: env1,
+                                    },
+                                    fst_pos,
+                                );
 
-                            Ok(Closure { body, env: env1 })
+                                Closure {
+                                    body: internals::naked_to_custom(),
+                                    env: Environment::new(),
+                                }
+                            }
                         }
-                        Term::CustomContract(ctr) => {
-                            let body = if let BinaryOp::ContractApply = b_op {
-                                mk_app!(internals::prepare_custom_contract(), ctr.clone())
-                                    .with_pos(pos1)
-                            } else {
-                                ctr.clone()
-                            };
+                        Term::CustomContract(ctr) => Closure {
+                            body: ctr.clone(),
+                            env: env1,
+                        },
+                        Term::Record(..) => {
+                            // Prepare the stack to represent the evaluation context `[.] t1` and
+                            // proceed with `$record_contract`
+                            self.stack.push_arg(
+                                Closure {
+                                    body: RichTerm {
+                                        term: t1,
+                                        pos: pos1,
+                                    },
+                                    env: env1,
+                                },
+                                fst_pos,
+                            );
 
-                            Ok(Closure { body, env: env1 })
+                            Closure {
+                                body: internals::record_contract(),
+                                env: Environment::new(),
+                            }
                         }
-                        Term::Record(..) => {
-                            let pos1_inh = pos1.into_inherited();
+                        _ => return mk_type_error!("Contract", 1, t1, pos1),
+                    };
 
-                            // Convert the record to the builtin contract implementation
-                            // `$record_contract <record>`.
-                            let as_custom = mk_app!(
-                                internals::record_contract(),
-                                RichTerm {
-                                    term: t1,
-                                    pos: pos1
-                                }
-                            )
-                            .with_pos(pos1_inh);
+                    Ok(functoid)
+                } else {
+                    mk_type_error!("Label", 2, t2.into(), pos2)
+                }
+            }
+            BinaryOp::LabelWithErrorData => {
+                // We need to extract plain values from a Nickel data structure, which most likely
+                // contains closures at least, even if it's fully evaluated. As for serialization,
+                // we thus need to fully substitute all variables first.
+                let t1 = subst(
+                    &self.cache,
+                    RichTerm {
+                        term: t1,
+                        pos: pos1,
+                    },
+                    &self.initial_env,
+                    &env1,
+                )
+                .term
+                .into_owned();
 
-                            let body = if let BinaryOp::ContractApply = b_op {
-                                // Convert the record to the builtin contract implementation
-                                // `$prepare_custom_contract ($record_contract <record>)`.
-                                mk_app!(internals::prepare_custom_contract(), as_custom)
-                                    .with_pos(pos1_inh)
-                            } else {
-                                as_custom
-                            };
+                let t2 = t2.into_owned();
+
+                let Term::Lbl(mut label) = t2 else {
+                    return mk_type_error!("Label", 2, t2.into(), pos2);
+                };
 
-                            Ok(Closure { body, env: env1 })
+                if let Term::Record(mut record_data) = t1 {
+                    if let Some(Term::Str(msg)) = record_data
+                        .fields
+                        .remove(&LocIdent::from("message"))
+                        .and_then(|field| field.value)
+                        .map(|v| v.term.into_owned())
+                    {
+                        label = label.with_diagnostic_message(msg.into_inner());
+                    }
+
+                    if let Some(notes_term) = record_data
+                        .fields
+                        .remove(&LocIdent::from("notes"))
+                        .and_then(|field| field.value)
+                    {
+                        if let Term::Array(array, _) = notes_term.into() {
+                            let notes = array
+                                .into_iter()
+                                .map(|element| {
+                                    let term = element.term.into_owned();
+
+                                    if let Term::Str(s) = term {
+                                        Ok(s.into_inner())
+                                    } else {
+                                        mk_type_error!(
+                                            "String (notes)",
+                                            1,
+                                            term.into(),
+                                            element.pos
+                                        )
+                                    }
+                                })
+                                .collect::<Result<Vec<_>, _>>()?;
+
+                            label = label.with_diagnostic_notes(notes);
                         }
-                        _ => mk_type_error!("Contract", 1, t1, pos1),
                     }
+
+                    Ok(Closure::atomic_closure(RichTerm::new(
+                        Term::Lbl(label),
+                        pos2,
+                    )))
                 } else {
-                    mk_type_error!("Label", 2, t2.into(), pos2)
+                    mk_type_error!("Record", 1, t1.into(), pos1)
                 }
             }
             BinaryOp::Unseal => {
diff --git a/core/src/term/mod.rs b/core/src/term/mod.rs
index 595c6875..acf87974 100644
--- a/core/src/term/mod.rs
+++ b/core/src/term/mod.rs
@@ -1501,6 +1501,13 @@ pub enum UnaryOp {
     /// type constructor for custom contracts.
     ContractCustom,
 
+    /// After applying a custom contract (or a builtin contract), the result is either `'Ok value`
+    /// or `'Error err_data`. This primop post-processes this return value (the first argument) to
+    /// either produce `value` in the first case or to attach the error data to the label (the
+    /// second argument, taken from the stack, as this op isn't strict in the label) and blame in
+    /// the second.
+    ContractPostprocessResult,
+
     /// The cosinus function.
     NumberArcCos,
 
@@ -1580,6 +1587,7 @@ impl fmt::Display for UnaryOp {
 
             PatternBranch => write!(f, "pattern_branch"),
             ContractCustom => write!(f, "contract/custom"),
+            ContractPostprocessResult => write!(f, "contract/postprocess_result"),
 
             NumberArcCos => write!(f, "number/arccos"),
             NumberArcSin => write!(f, "number/arcsin"),
@@ -1732,6 +1740,9 @@ pub enum BinaryOp {
     /// [Self::ContractCheck] preserves the immediate/delayed part of the called contract.
     ContractCheck,
 
+    /// Take a record of type `{message | String | optional, notes | String | optional}`.
+    LabelWithErrorData,
+
     /// Unseal a sealed term.
     ///
     /// See [`BinaryOp::Seal`].
@@ -1883,6 +1894,7 @@ impl fmt::Display for BinaryOp {
             GreaterOrEq => write!(f, "(>=)"),
             ContractApply => write!(f, "contract/apply"),
             ContractCheck => write!(f, "contract/check"),
+            LabelWithErrorData => write!(f, "label/with_error_data"),
             Unseal => write!(f, "unseal"),
             LabelGoField => write!(f, "label/go_field"),
             RecordInsert {
diff --git a/core/src/typecheck/mod.rs b/core/src/typecheck/mod.rs
index 1d70ac5f..ff5823dc 100644
--- a/core/src/typecheck/mod.rs
+++ b/core/src/typecheck/mod.rs
@@ -2383,6 +2383,7 @@ pub fn subsumption(
     checked: UnifType,
 ) -> Result<(), UnifError> {
     let inferred_inst = instantiate_foralls(state, &mut ctxt, inferred, ForallInst::UnifVar);
+    let checked = checked.into_root(state.table);
     match (inferred_inst, checked) {
         (
             UnifType::Concrete {
diff --git a/core/src/typecheck/operation.rs b/core/src/typecheck/operation.rs
index d6b450c6..0d5ed874 100644
--- a/core/src/typecheck/operation.rs
+++ b/core/src/typecheck/operation.rs
@@ -273,7 +273,12 @@ pub fn get_uop_type(
         }
         // <custom_contract_type()> -> Dyn
         UnaryOp::ContractCustom => (custom_contract_type(), mk_uniftype::dynamic()),
-        // Num -> Num
+        // <custom_contract_ret_type()> -> Dyn -> Dyn
+        UnaryOp::ContractPostprocessResult => (
+            custom_contract_ret_type(),
+            mk_uty_arrow!(mk_uniftype::dynamic(), mk_uniftype::dynamic()),
+        ),
+        // Number -> Number
         UnaryOp::NumberCos
         | UnaryOp::NumberSin
         | UnaryOp::NumberTan
@@ -316,6 +321,13 @@ pub fn get_bop_type(
             mk_uniftype::dynamic(),
             mk_uty_arrow!(mk_uniftype::dynamic(), custom_contract_ret_type()),
         ),
+        // Ideally: <error_data_type()> -> Label -> Dyn
+        // Currently: <error_data_type()> -> Dyn -> Dyn
+        BinaryOp::LabelWithErrorData => (
+            error_data_type(),
+            mk_uniftype::dynamic(),
+            mk_uniftype::dynamic(),
+        ),
         // Sym -> Dyn -> Dyn -> Dyn
         BinaryOp::Unseal => (
             mk_uniftype::sym(),
@@ -631,6 +643,31 @@ pub fn custom_contract_type() -> UnifType {
 /// |]
 /// ```
 pub fn custom_contract_ret_type() -> UnifType {
+    mk_uty_enum!(
+        ("Ok", mk_uniftype::dynamic()),
+        // /!\ IMPORTANT
+        //
+        // During typechecking, `TypeF::Flat` all have been transformed to `UnifType::Contract(..)`
+        // with their associated environment. Generating a `TypeF::Flat` at this point will panic
+        // in the best case (we should catch that), or have incorrect behavior in the worst case.
+        // Do not turn this into `UnifType::concrete(TypeF::Flat(..))`.
+        ("Error", error_data_type())
+    )
+}
+
+/// The type of error data that can be returned by a custom contract:
+///
+/// ```nickel
+/// {
+///   message
+///     | String
+///     | optional,
+///   notes
+///     | Array String
+///     | optional
+/// }
+/// ```
+fn error_data_type() -> UnifType {
     use crate::term::make::builder;
 
     let error_data = builder::Record::new()
@@ -649,17 +686,5 @@ pub fn custom_contract_ret_type() -> UnifType {
         .optional()
         .no_value();
 
-    mk_uty_enum!(
-        ("Ok", mk_uniftype::dynamic()),
-        // /!\ IMPORTANT
-        //
-        // During typechecking, `TypeF::Flat` all have been transformed to `UnifType::Contract(..)`
-        // with their associated environment. Generating a `TypeF::Flat` at this point will panic
-        // in the best case (we should catch that), or have incorrect behavior in the worst case.
-        // Do not turn this into `UnifType::concrete(TypeF::Flat(..))`.
-        (
-            "Error",
-            UnifType::Contract(error_data.build(), SimpleTermEnvironment::new())
-        )
-    )
+    UnifType::Contract(error_data.build(), SimpleTermEnvironment::new())
 }
diff --git a/core/stdlib/internals.ncl b/core/stdlib/internals.ncl
index 66b8d004..280aa6f1 100644
--- a/core/stdlib/internals.ncl
+++ b/core/stdlib/internals.ncl
@@ -385,51 +385,6 @@
   # are no extra fields, so we can just ignore them and return the value as is.
   "$empty_tail" = fun _extra_fields _label value => 'Ok value,
 
-  # Take a a partial identity Label -> Dyn -> Dyn that can then be passed
-  # applied as a normal funtion in the implementation of `contract/apply`.
-  "$prepare_custom_contract" = fun custom_contract label value =>
-    custom_contract label value
-    |> match {
-      'Ok value => value,
-      'Error { message ? null, notes ? [], .. } =>
-        let label =
-          if std.is_string message then
-            %label/with_message% message label
-          else
-            label
-        in
-
-        let label =
-          if notes != [] && std.is_array notes then
-            %label/with_notes% (%force% notes) label
-          else
-            label
-        in
-
-        %blame% label,
-      # The contract of `std.contract.custom` should guarantee that we
-      # never reach this case. However, nothing prevents user from using
-      # `%contract/custom%` directly, so we still try to handle a misbehaving
-      # custom contract gracefully.
-      value =>
-        %blame%
-          (
-            %label/with_notes%
-              (
-                %force%
-                  (
-                    %trace%
-                      value
-                      [
-                        "The implementation of this custom contract returned an invalid result (which must be of type `[| 'Ok Dyn, 'Error {..} |]`)",
-                        "Please check the implementation of the contract that has been broken"
-                      ]
-                  )
-              )
-              label
-          )
-    },
-
   # Dual of prepare_custom_contract. Turns a naked function of a label and a
   # value to a custom contract-like representation (that is, a function
   # returning an enum - however the result is still a naked function, which must
diff --git a/doc/manual/typing.md b/doc/manual/typing.md
index 8c280bc1..68894b21 100644
--- a/doc/manual/typing.md
+++ b/doc/manual/typing.md
@@ -255,6 +255,90 @@ The following type constructors are available:
   }
   ```
 
+### Subtyping
+
+While distinct types are usually incompatible, some types might actually be
+safely converted to some other types. For example, `{foo = 5}` is inferred to
+have type `{foo : Number}` by default, but is also a valid value of type `{_ :
+Number}`. The latter type is less precise, but might be required for dynamic
+dictionary operations such as `std.record.insert`:
+
+```nickel
+let extended : { _ : Number } =
+  let initial : { foo : Number } = { foo = 5 } in
+  std.record.insert "bar" 5 initial in
+extended
+```
+
+In this example, there is a silent conversion from `{foo : Number}` to `{_ :
+Number}`. This is safe because `foo` is of type `Number`, and it's the only
+field, which means that a value of type `{foo : Number}` is effectively
+dictionary of numbers. In the typing jargon, `{foo : Number}` is said to be a
+*subtype* of `{_ : Number}`. We will write `T <: U` to say that `T` is a subtype
+of `U`: whenever a value of type `U` is expected, we can use a value of type `T`
+as well.
+
+Currently, Nickel supports the following subtyping rule which generalizes the
+example above:
+
+- **Record/Dictionary** subtyping :
+  `{ field1 : T1, ..., fieldn : Tn } <: { _ : T }` if for each `i`, `Ti <: T`
+  That is, a record type is a subtype of a dictionary type if the type of each
+  field is a subtype of the type of dictionary elements.
+
+  Example:
+
+  ```nickel
+  let occurrences : {a : Number, b : Number, c : Number} = {a = 1, b = 3, c = 0} in
+  std.record.map (fun char count => count + 1) occurrences : {_ : Number}
+  ```
+
+Subtyping extends to type constructors in the following way:
+
+- **Equality** : a type is trivially a subtype of itself, that is `T <: T`.
+- **Array**: `Array T <: Array U` if `T <: U`.
+
+  Example:
+
+  ```nickel
+  let block : _ =
+      let array_of_records : Array {a : Number} = [{a = 5}] in
+      let inject_b : Array {_ : Number} -> Array {_ : Number} =
+          std.array.map (fun a => std.record.insert "b" 5 a)
+      in
+
+      inject_b array_of_records
+  in block
+  ```
+
+  Here, `Array {a : Number}` is accepted where `Array {_ : Number}` is expected,
+  because `{a : Number} <: { _ : Number }`.
+- **Dictionary**: similarly, `{ _ : T } <: { _ : U }` if `T <: U`.
+
+  Example:
+
+  ```nickel
+  let block : _ =
+    let dict_of_records : { _ : {a : Number}} = {r = {a = 5}} in
+    let inject_b : { _ : {_ : Number}} -> { _ : {_ : Number}}
+      = std.record.map (fun _ x => std.record.insert "b" 5 x)
+    in
+
+    inject_b dict_of_records in
+  block
+  ```
+
+  Here, `{_ : {a : Number}}` is accepted where `{_ : {_ : Number}}` is expected,
+  because `{a : Number} <: { _ : Number }`.
+
+**Remark**: if you've used languages with subtyping before, you might expect the
+presence of a rule for function types, namely that `T -> U <: S -> V` if `S <:
+T` and `U <: V`. This is not the case in Nickel, for various technical reasons.
+However, you can work around this limitation by expanding a given function:
+If `f` has type `T -> U`, but a value of type `S -> V` is required,
+use `fun x => f x` instead, which has the same runtime behavior
+but isn't typed in the same way.
+
 ### Polymorphism
 
 #### Type polymorphism


Test patch:
diff --git a/cli/tests/snapshot/snapshots/snapshot__eval_stderr_function_contract_domain_violation.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__eval_stderr_function_contract_domain_violation.ncl.snap
index d2cfb04c..43a7a7ed 100644
--- a/cli/tests/snapshot/snapshots/snapshot__eval_stderr_function_contract_domain_violation.ncl.snap
+++ b/cli/tests/snapshot/snapshots/snapshot__eval_stderr_function_contract_domain_violation.ncl.snap
@@ -15,16 +15,8 @@ error: contract broken by the caller
 1 │ "a"
   │ --- evaluated to this value
 
-note: 
-  ┌─ [INPUTS_PATH]/errors/function_contract_domain_violation.ncl:5:31
-  │
-5 │ let Foo = Number -> Number in ((fun x => x) | Foo) "a"
-  │                               ------------------------ (1) calling <func>
-
 note: 
   ┌─ [INPUTS_PATH]/errors/function_contract_domain_violation.ncl:5:32
   │
 5 │ let Foo = Number -> Number in ((fun x => x) | Foo) "a"
-  │                                ------------ (2) calling <func>
-
-
+  │                                ^^^^^^^^^^^^ While calling to x
diff --git a/cli/tests/snapshot/snapshots/snapshot__eval_stderr_record_forall_constraints_contract.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__eval_stderr_record_forall_constraints_contract.ncl.snap
index 662b27d2..5717e331 100644
--- a/cli/tests/snapshot/snapshots/snapshot__eval_stderr_record_forall_constraints_contract.ncl.snap
+++ b/cli/tests/snapshot/snapshots/snapshot__eval_stderr_record_forall_constraints_contract.ncl.snap
@@ -16,15 +16,3 @@ note:
   │
 3 │ let f | forall r. { ; r } -> { x: Number; r } = fun r => %record/insert% "x" r 1 in f { x = 0 }
   │                                                          ^^^^^^^^^^^^^^^^^^^^^^^ While calling to r
-
-note: 
-  ┌─ [INPUTS_PATH]/errors/record_forall_constraints_contract.ncl:3:85
-  │
-3 │ let f | forall r. { ; r } -> { x: Number; r } = fun r => %record/insert% "x" r 1 in f { x = 0 }
-  │                                                                                     ----------- (1) calling f
-
-note: 
-  ┌─ [INPUTS_PATH]/errors/record_forall_constraints_contract.ncl:3:49
-  │
-3 │ let f | forall r. { ; r } -> { x: Number; r } = fun r => %record/insert% "x" r 1 in f { x = 0 }
-  │                                                 -------------------------------- (2) calling <func>
diff --git a/core/tests/integration/inputs/typecheck/chaining_dictionary_function_record_dictionary_subtyping.ncl b/core/tests/integration/inputs/typecheck/chaining_dictionary_function_record_dictionary_subtyping.ncl
new file mode 100644
index 00000000..a9fa59c1
--- /dev/null
+++ b/core/tests/integration/inputs/typecheck/chaining_dictionary_function_record_dictionary_subtyping.ncl
@@ -0,0 +1,8 @@
+# test.type = 'error'
+# [test.metadata]
+# error = 'EvalError::BlameError'
+let x = {foo = 1, bar = 2} in
+let y = std.record.insert "baz" 5 x in
+let _ = std.record.get "foo" y in
+let z = std.record.remove "foo" y in
+std.record.get "foo" z