Repository: wilkerlucio/pathom
Pull request #109 (2019-08-12 22:58:35)

Fix holding external waits


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8e12e25..ad4f0139 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,13 @@
 # Change Log
 
-## [2.2.20-DEV]
+## [2.2.21-DEV]
+- Add a mechanism to retry long running external waiting on connect parallel reader.
+- Fixed issue that made resolvers stuck when depending on a external data in middle of resolver chain
+- Fixed counting behavior on max-key-iterations
+- Reduced default max-key-iterations to 5
+- Support `::p/final` on `p/map-reader` for short circuit processing
+
+## [2.2.20]
 - Fixed issue #105, when a resolver gets an error but another concurrent gets a valid value after, the later must be used as the response
 
 ## [2.2.19]
diff --git a/README.md b/README.md
index 072b86ce..75f2fb71 100644
--- a/README.md
+++ b/README.md
@@ -13,13 +13,13 @@ And all documentation here assumes you understand the Om.next query syntax.
 The library has a number of different usage models. One of the most commonly
 used features is Connect.  The basic idea is as follows:
 
-- You define *resolvers* that can take simple inputs (e.g. entity ID) and turn
+. You define *resolvers* that can take simple inputs (e.g. entity ID) and turn
 that into a set of outputs.
-- Your resolvers implies "edges" by outputting IDs that can be further resolved
+. Your resolvers implies "edges" by outputting IDs that can be further resolved
 by other resolvers.
 
 The Connect features support advanced features for being able to then traverse
-this graph via queries in useful ways.
+this graph via queries is useful ways.
 
 For example, here is how you might generate the support for queries about
 people and their addresses.
@@ -32,8 +32,7 @@ of the real database):
 (ns sample-query-processing
   (:require
     [com.wsscode.pathom.core :as p]
-    [com.wsscode.pathom.connect :as pc]
-    [clojure.core.async :refer [<!!]]))
+    [com.wsscode.pathom.connect :as pc]))
 
 ;; How to go from :person/id to that person's details
 (pc/defresolver person-resolver [env {:keys [person/id] :as params}]
@@ -73,7 +72,7 @@ of the real database):
 ;; env can have anything you want in it (e.g. a Datomic/SQL connection, network service endpoint, etc.)
 ;; the concurrency is handled though core.async, so you have to read the channel to get the output
 (<!! (parser {} [{[:person/id 1] [:person/name {:person/address [:address/city]}]}]))
-; => {[:person/id 1] {:person/name "Tom" :person/address {:address/city "Salem"}}}
+; => {[:person/id 1] {:person/id 1 :person/name "Tom" :person/address {:address/city "Salem}}}
 ```
 
 In the example above hopefully you can start to see the general power: Small resolvers
diff --git a/package-lock.json b/package-lock.json
index 86c73ddd..457aa517 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -987,9 +987,9 @@
       "integrity": "sha512-vm3/XWXfWtRua0FkUyEHBZy8kCPjErNBT9fJx8Zvs+U6zjqPbTUOpkaoum3O5uiA8sm+yNMHXfYkTUHFoMxFNA=="
     },
     "lodash-es": {
-      "version": "4.17.14",
-      "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.14.tgz",
-      "integrity": "sha512-7zchRrGa8UZXjD/4ivUWP1867jDkhzTG2c/uj739utSd7O/pFFdxspCemIFKEEjErbcqRzn8nKnGsi7mvTgRPA=="
+      "version": "4.17.10",
+      "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.17.10.tgz",
+      "integrity": "sha512-iesFYPmxYYGTcmQK0sL8bX3TGHyM6b2qREaB4kamHfQyfPJP0xgoGxp19nsH16nsfquLdiyKyX3mQkfiSGV8Rg=="
     },
     "lodash.isequal": {
       "version": "4.5.0",
diff --git a/project.clj b/project.clj
index bd35b792..92523506 100644
--- a/project.clj
+++ b/project.clj
@@ -1,4 +1,4 @@
-(defproject com.wsscode/pathom "2.2.20"
+(defproject com.wsscode/pathom "2.2.21-SNAPSHOT"
   :description "A Clojure library designed to provide a collection of helper functions to support Clojure(script) graph parsers using\nom.next graph syntax."
   :url "https://github.com/wilkerlucio/pathom"
   :license {:name "MIT"
diff --git a/src/com/wsscode/pathom/connect.cljc b/src/com/wsscode/pathom/connect.cljc
index 87385a2c..9c63579c 100644
--- a/src/com/wsscode/pathom/connect.cljc
+++ b/src/com/wsscode/pathom/connect.cljc
@@ -1131,14 +1131,15 @@
      out
 
      ::pp/response-stream
-     (let [ch  (async/chan 10)
-           key (-> env :ast :key)
+     (let [ch     (async/chan 10)
+           key    (-> env :ast :key)
            params (p/params env)
-           env (assoc env ::plan-path plan)]
+           env    (assoc env ::plan-path plan)]
        (go
          (loop [[step & tail] plan
                 failed-resolvers {}
-                out-left         out]
+                out-left         out
+                waiting          waiting]
            (if step
              (let [[key' resolver-sym] step
                    {::keys [cache? batch? input] :or {cache? true} :as resolver}
@@ -1181,8 +1182,11 @@
                                         [plan failed-resolvers out'])))))]
 
                (cond
+                 (identical? ::pp/watch-pending-timeout response)
+                 (recur plan failed-resolvers out-left (disj waiting key'))
+
                  (identical? ::watch-ready response)
-                 (recur tail failed-resolvers (set/difference out-left (set (keys (p/entity env)))))
+                 (recur tail failed-resolvers (set/difference out-left (set (keys (p/entity env)))) waiting)
 
                  (map? response)
                  (let [response (dissoc response ::env)]
@@ -1195,10 +1199,10 @@
                                       ::sym      resolver-sym})
                        (>! ch {::pp/provides       out-provides
                                ::pp/response-value response})
-                       (recur tail failed-resolvers (set/difference out-left out-provides)))
+                       (recur tail failed-resolvers (set/difference out-left out-provides) waiting))
 
                      (if-let [[plan failed-resolvers out'] (<! (replan response (ex-info "Insufficient resolver output" {::pp/response-value response :key key'})))]
-                       (recur plan failed-resolvers out')
+                       (recur plan failed-resolvers out' waiting)
                        (let [err (ex-info "Insufficient resolver output" {::pp/response-value response :key key'})]
                          (p/swap-entity! env #(merge response %))
                          (if (seq tail)
@@ -1215,7 +1219,7 @@
 
                  (p.async/error? response)
                  (if-let [[plan failed-resolvers out'] (<! (replan {} response))]
-                   (recur plan failed-resolvers out')
+                   (recur plan failed-resolvers out' waiting)
                    (do
                      (pt/trace env {::pt/event ::resolver-error
                                     :key       key
@@ -1231,7 +1235,7 @@
 
                  :else
                  (if-let [[plan failed-resolvers out'] (<! (replan {} (ex-info "Invalid resolve response" {::pp/response-value response})))]
-                   (recur plan failed-resolvers out')
+                   (recur plan failed-resolvers out' waiting)
                    (let [err (ex-info "Invalid resolve response" {::pp/response-value response})]
                      (pt/trace env {::pt/event          ::invalid-resolve-response
                                     :key                key
diff --git a/src/com/wsscode/pathom/core.cljc b/src/com/wsscode/pathom/core.cljc
index d7572a40..691318a4 100644
--- a/src/com/wsscode/pathom/core.cljc
+++ b/src/com/wsscode/pathom/core.cljc
@@ -625,10 +625,10 @@
     (if (contains? entity (:key ast))
       (let [v (get entity (:key ast))]
         (if (sequential? v)
-          (if query
+          (if (and query (not (::final (meta v))))
             (join-seq env v)
             v)
-          (if (and (map? v) query)
+          (if (and (map? v) query (not (::final (meta v))))
             (join v env)
             v)))
       ::continue)))
@@ -1096,7 +1096,11 @@
 
   ::pc/async-request-cache-ch-size - Pathom uses internally a queue to avoid concurrency
   issues with concurrency, each request gets its own channel, so you can consider this
-  size needs to accomodate the max parallelism for a single query. Default: 1024
+  size needs to accommodate the max parallelism for a single query. Default: 1024
+
+  ::pp/external-wait-ignore-timeout - Sometimes external waits get stuck because a concurrency
+  problem, this timeout will ignore external waits after some time so the request can
+  go on. Default: 3000
 
   ::pp/max-key-iterations - there is a loop that happens when processing attributes in
   parallel, this loop will cause multiple iterations to happen in order for a single
diff --git a/src/com/wsscode/pathom/parser.cljc b/src/com/wsscode/pathom/parser.cljc
index 17d087a2..b816af7c 100644
--- a/src/com/wsscode/pathom/parser.cljc
+++ b/src/com/wsscode/pathom/parser.cljc
@@ -11,7 +11,8 @@
 
 (when p.misc/INCLUDE_SPECS
   (s/def ::max-key-iterations int?)
-  (s/def ::processing-recheck-timer pos-int?))
+  (s/def ::processing-recheck-timer pos-int?)
+  (s/def ::external-wait-ignore-timeout (s/nilable pos-int?)))
 
 (declare expr->ast)
 
@@ -280,7 +281,9 @@
                 (recur (assoc res (ast->out-key ast) value) tail))
               res)))))))
 
-(defn watch-pending-key [{::keys [key-watchers] :as env} key]
+(defn watch-pending-key [{::keys [key-watchers external-wait-ignore-timeout]
+                          :or    {external-wait-ignore-timeout 3000}
+                          :as    env} key]
   (let [ch (async/chan)]
     (swap! key-watchers update key conj ch)
     (go
@@ -291,7 +294,19 @@
         (trace env {::pt/event ::flush-watcher-safeguard :key key})
         (async/put! ch {::provides #{key}})
         (async/close! ch)))
-    ch))
+
+    (if external-wait-ignore-timeout
+      (go
+        (let [timer (async/timeout external-wait-ignore-timeout)
+              [res ch] (async/alts! [ch timer]
+                         :priority true)]
+          (if (= ch timer)
+            (do
+              (pt/trace env {::pt/event                     ::watch-pending-timeout
+                             ::external-wait-ignore-timeout external-wait-ignore-timeout})
+              {::error ::watch-pending-timeout})
+            res)))
+      ch)))
 
 ; urh, ugly copy from core but needed to avoid dep cycles
 (defn- process-error [{:com.wsscode.pathom.core/keys [process-error] :as env} e]
@@ -374,7 +389,7 @@
                     :key            pkey
                     ::watcher-count (count watchers)})
         (doseq [out watchers]
-          (async/put! out {::provides #{pkey}
+          (async/put! out {::provides provides
                            ::error    error})
           (async/close! out))
         (swap! key-watchers dissoc pkey)))))
@@ -391,7 +406,8 @@
   "This is used for merging new parsed attributes from entity, works like regular merge but if the value from the right
   direction is not found, then the previous value will be kept."
   [x y]
-  (if (identical? y :com.wsscode.pathom.core/reader-error)
+  (if (or (identical? y :com.wsscode.pathom.core/reader-error)
+          (identical? y :com.wsscode.pathom.core/not-found))
     x
     y))
 
@@ -462,16 +478,18 @@
                   [res
                    (into #{} (remove provides') waiting)
                    processing
-                   (reduce (fn [iter {:keys [key]}] (update iter key (fnil inc 0))) key-iterations next-children)
+                   key-iterations
                    next-children])))
             [res waiting (into #{} (remove (comp #{p} ::process-channel)) processing) key-iterations []]))))))
 
+(def zero-inc (fnil inc 0))
+
 (defn call-parallel-parser
   [{:keys [read mutate]}
    {::keys                        [waiting key-watchers max-key-iterations
                                    key-process-timeout key-process-timeout-step]
     :com.wsscode.pathom.core/keys [entity-path-cache path]
-    :or                           {max-key-iterations 10}
+    :or                           {max-key-iterations 5}
     :as                           env}
    tx]
   (go-catch
@@ -538,7 +556,7 @@
                         env tx ast
                         key-watchers
                         res waiting processing
-                        read mutate key-iterations tail)]
+                        read mutate (update key-iterations out-key zero-inc) tail)]
                   (recur res waiting processing key-iterations tail))))
 
             ; waiting for results
diff --git a/src/com/wsscode/pathom/trace.cljc b/src/com/wsscode/pathom/trace.cljc
index 3f3ae9dc..6d19195f 100644
--- a/src/com/wsscode/pathom/trace.cljc
+++ b/src/com/wsscode/pathom/trace.cljc
@@ -90,8 +90,11 @@
                 :keys  [key]
                 :as    row}]
           (case event
-            (:com.wsscode.pathom.parser/parse-loop :com.wsscode.pathom.core/trace-plugin)
-            (update x :response #(merge (select-keys row [::relative-timestamp ::duration :com.wsscode.pathom.core/path]) %))
+            (:com.wsscode.pathom.core/trace-plugin
+              :com.wsscode.pathom.parser/parse-loop)
+            (update x :response #(merge (select-keys row [::duration
+                                                          ::relative-timestamp
+                                                          :com.wsscode.pathom.core/path]) %))
 
             :com.wsscode.pathom.parser/process-key
             (let [next-path (conj path key)]
@@ -100,7 +103,8 @@
                 (-> x
                     (assoc-in [:response :com.wsscode.pathom.core/path] path)
                     (assoc-in [:response ::children key]
-                      (-> (merge (trace->tree* paths next-path) (select-keys row [::relative-timestamp :key]))
+                      (-> (merge (trace->tree* paths next-path) (select-keys row [:key
+                                                                                  ::relative-timestamp]))
                           (assoc :com.wsscode.pathom.core/path next-path)))
                     (update :visited conj next-path))))
 
@@ -110,45 +114,103 @@
                 (fn [x i]
                   (let [next-path (conj path i)]
                     (assoc-in x [:response ::children i]
-                      (-> (merge (trace->tree* paths next-path) (select-keys row [::relative-timestamp :key]) {:key i})
+                      (-> (merge (trace->tree* paths next-path) (select-keys row [:key
+                                                                                  ::relative-timestamp]) {:key i})
                           (assoc :com.wsscode.pathom.core/path next-path)))))
                 x
                 (range count)))
 
-            (:com.wsscode.pathom.parser/async-return :com.wsscode.pathom.parser/skip-wait-key :com.wsscode.pathom.parser/call-read
-              :com.wsscode.pathom.parser/skip-resolved-key :com.wsscode.pathom.parser/external-wait-key)
-            (update-in x [:response ::children key ::details] (fnil conj []) (select-keys row [::event ::label ::style ::relative-timestamp :key]))
-
-            (:com.wsscode.pathom.parser/trigger-reader-retry :com.wsscode.pathom.parser/trigger-recheck-schedule)
+            (:com.wsscode.pathom.parser/async-return
+              :com.wsscode.pathom.parser/call-read
+              :com.wsscode.pathom.parser/external-wait-key
+              :com.wsscode.pathom.parser/skip-resolved-key
+              :com.wsscode.pathom.parser/skip-wait-key)
+            (update-in x [:response ::children key ::details] (fnil conj [])
+              (select-keys row [:key
+                                ::event
+                                ::label
+                                ::style
+                                ::relative-timestamp]))
+
+            (:com.wsscode.pathom.parser/trigger-reader-retry
+              :com.wsscode.pathom.parser/trigger-recheck-schedule)
             (update-in x [:response ::details] (fnil conj [])
-              (select-keys row [::event ::label ::style ::relative-timestamp :com.wsscode.pathom.parser/processing :key]))
+              (select-keys row [:key
+                                ::event
+                                ::label
+                                ::relative-timestamp
+                                ::style
+                                :com.wsscode.pathom.parser/processing]))
 
             :com.wsscode.pathom.parser/max-iterations-reached
-            (update-in x [:response ::children key ::details] (fnil conj []) (select-keys row [::event ::label ::style ::relative-timestamp :com.wsscode.pathom.parser/max-key-iterations]))
-
-            (:com.wsscode.pathom.parser/process-pending :com.wsscode.pathom.parser/reset-loop
-              :com.wsscode.pathom.parser/flush-watchers-loop ::trace-done)
-            (update-in x [:response ::details] (fnil conj []) (select-keys row [::event ::label ::style ::relative-timestamp :com.wsscode.pathom.parser/provides :com.wsscode.pathom.parser/merge-result?
-                                                                                :com.wsscode.pathom.parser/loop-keys]))
+            (update-in x [:response ::children key ::details] (fnil conj [])
+              (select-keys row [::event
+                                ::label
+                                ::style
+                                ::relative-timestamp
+                                :com.wsscode.pathom.parser/max-key-iterations]))
+
+            (::trace-done
+              :com.wsscode.pathom.parser/flush-watchers-loop
+              :com.wsscode.pathom.parser/process-pending
+              :com.wsscode.pathom.parser/reset-loop)
+            (update-in x [:response ::details] (fnil conj [])
+              (select-keys row [::event
+                                ::label
+                                ::relative-timestamp
+                                ::style
+                                :com.wsscode.pathom.parser/loop-keys
+                                :com.wsscode.pathom.parser/merge-result?
+                                :com.wsscode.pathom.parser/provides]))
 
             :com.wsscode.pathom.parser/merge-result
             (reduce
               (fn [x key]
-                (update-in x [:response ::children key ::details] (fnil conj []) (select-keys row [::event ::label ::style ::relative-timestamp])))
+                (update-in x [:response ::children key ::details] (fnil conj [])
+                  (select-keys row [::event
+                                    ::label
+                                    ::relative-timestamp
+                                    ::style])))
               x
               (keys (:com.wsscode.pathom.parser/response-value row)))
 
-            (:com.wsscode.pathom.connect/compute-plan :com.wsscode.pathom.connect/waiting-resolver :com.wsscode.pathom.connect/schedule-resolver
-              :com.wsscode.pathom.connect/call-resolver-with-cache :com.wsscode.pathom.connect/call-resolver :com.wsscode.pathom.core/parallel-sequence-loop
-              :com.wsscode.pathom.connect/call-resolver-batch :com.wsscode.pathom.connect/batch-items-ready :com.wsscode.pathom.connect/batch-result-error :com.wsscode.pathom.connect/batch-result-ready
-              :com.wsscode.pathom.connect/merge-resolver-response :com.wsscode.pathom.connect/resolver-error :com.wsscode.pathom.connect/invalid-resolve-response)
+            (:com.wsscode.pathom.connect/batch-items-ready
+              :com.wsscode.pathom.connect/batch-result-error
+              :com.wsscode.pathom.connect/batch-result-ready
+              :com.wsscode.pathom.connect/call-resolver
+              :com.wsscode.pathom.connect/call-resolver-batch
+              :com.wsscode.pathom.connect/call-resolver-with-cache
+              :com.wsscode.pathom.connect/compute-plan
+              :com.wsscode.pathom.connect/invalid-resolve-response
+              :com.wsscode.pathom.connect/merge-resolver-response
+              :com.wsscode.pathom.connect/resolver-error
+              :com.wsscode.pathom.connect/schedule-resolver
+              :com.wsscode.pathom.connect/waiting-resolver
+              :com.wsscode.pathom.core/parallel-sequence-loop
+              :com.wsscode.pathom.parser/watch-pending-timeout)
             (update-in x [:response ::details] (fnil conj [])
-              (select-keys row [::event ::label ::style ::relative-timestamp ::duration :com.wsscode.pathom.connect/waiting-key :com.wsscode.pathom.connect/input-data
-                                :com.wsscode.pathom.parser/provides :com.wsscode.pathom.connect/sym :com.wsscode.pathom.core/error :com.wsscode.pathom.connect/items-count :com.wsscode.pathom.connect/plan :key]))
+              (select-keys row [:key
+                                ::duration
+                                ::event
+                                ::label
+                                ::relative-timestamp
+                                ::style
+                                :com.wsscode.pathom.connect/input-data
+                                :com.wsscode.pathom.connect/items-count
+                                :com.wsscode.pathom.connect/plan
+                                :com.wsscode.pathom.connect/sym
+                                :com.wsscode.pathom.connect/waiting-key
+                                :com.wsscode.pathom.core/error
+                                :com.wsscode.pathom.parser/external-wait-ignore-timeout
+                                :com.wsscode.pathom.parser/provides]))
 
             :com.wsscode.pathom.parser/value-return
             (-> x
-                (update-in [:response ::children key ::details] (fnil conj []) (select-keys row [::event ::label ::style ::relative-timestamp]))
+                (update-in [:response ::children key ::details] (fnil conj [])
+                  (select-keys row [::event
+                                    ::label
+                                    ::relative-timestamp
+                                    ::style]))
                 (update-in [:response ::children key] assoc ::duration (- relative-timestamp (get-in x [:response ::children key ::relative-timestamp]))))
 
             (trace-tree-collect x row)))


Test patch:
diff --git a/test/com/wsscode/pathom/connect_test.cljc b/test/com/wsscode/pathom/connect_test.cljc
index 8d619df4..7098d59b 100644
--- a/test/com/wsscode/pathom/connect_test.cljc
+++ b/test/com/wsscode/pathom/connect_test.cljc
@@ -1,7 +1,7 @@
 (ns com.wsscode.pathom.connect-test
   (:require [clojure.test :refer [is are testing]]
             #?(:clj
-               [com.wsscode.common.async-clj :refer [go-promise <!maybe <?]])
+               [com.wsscode.common.async-clj :refer [go-catch go-promise <!maybe <?]])
             [nubank.workspaces.core :refer [deftest]]
             [clojure.core.async :as async :refer [go <! <!!]]
             [com.wsscode.pathom.core :as p]
@@ -3183,21 +3183,27 @@
              :account/id  {#{:purchase/id} #{account}}})
          '#{})))
 
+(defonce quick-parser-trace* (atom []))
+
 #?(:clj
    (defn quick-parser [{::p/keys  [env]
                         ::pc/keys [register]} query]
-     (let [parser (p/parallel-parser {::p/env     (merge {::p/reader               [p/map-reader
+     (let [trace  (atom [])
+           parser (p/parallel-parser {::p/env     (merge {::p/reader               [p/map-reader
                                                                                     pc/parallel-reader
                                                                                     pc/open-ident-reader
                                                                                     p/env-placeholder-reader]
+                                                          ::pt/trace*              trace
                                                           ::p/placeholder-prefixes #{">"}}
                                                          env)
                                       ::p/mutate  pc/mutate-async
                                       ::p/plugins [(pc/connect-plugin {::pc/register register})
                                                    p/error-handler-plugin
                                                    p/request-cache-plugin
-                                                   p/trace-plugin]})]
-       (async/<!! (parser {} query)))))
+                                                   p/trace-plugin]})
+           res (async/<!! (parser {} query))]
+       (reset! quick-parser-trace* @trace)
+       res)))
 
 #?(:clj
    (deftest test-parallel-parser-with-connect
@@ -3398,4 +3404,32 @@
                     [:reg-nf1-a
                      :reg-nf1-b
                      ]))
-                {:reg-nf1-a 42 :reg-nf1-b ::p/not-found}))))))
+                {:reg-nf1-a 42 :reg-nf1-b ::p/not-found})))
+
+       (testing "external wait get notification when waiting for something in middle path"
+         (is (= (quick-parser {::pc/register [(pc/resolver 'a
+                                                {::pc/output [:a]}
+                                                (fn [env _]
+                                                  (go-catch
+                                                    (<! (async/timeout 200))
+                                                    (throw (ex-info "Er" {})))))
+
+                                              (pc/resolver 'b
+                                                {::pc/input  #{:a}
+                                                 ::pc/output [:b]}
+                                                (fn [env {:keys [a]}]
+                                                  {:b (inc a)}))
+
+                                              (pc/resolver 'c
+                                                {::pc/input  #{:b}
+                                                 ::pc/output [:c]}
+                                                (fn [env {:keys [b]}]
+                                                  {:c (inc b)}))]
+                               ::p/env       {::pp/external-wait-ignore-timeout 1000}}
+                  '[:c
+                    {:>/b [:b]}])
+                {:>/b                            {:b :com.wsscode.pathom.core/reader-error}
+                 :c                              :com.wsscode.pathom.core/reader-error
+                 :com.wsscode.pathom.core/errors {[:>/b
+                                                   :b] "class clojure.lang.ExceptionInfo: Er - {}"
+                                                  [:c] "class clojure.lang.ExceptionInfo: Er - {}"}}))))))
diff --git a/test/com/wsscode/pathom/core_test.cljc b/test/com/wsscode/pathom/core_test.cljc
index 336b060b..be5ce473 100644
--- a/test/com/wsscode/pathom/core_test.cljc
+++ b/test/com/wsscode/pathom/core_test.cljc
@@ -221,7 +221,19 @@
                                                           ::p/env (assoc env ::x 42)} env))}
                                 {::x (fn [env] (::x env))}]}
              '[{:data [:label ::x]}])
-           {:data {:label "value" ::x 42}}))))
+           {:data {:label "value" ::x 42}})))
+
+  (testing "optimized final return for values"
+    (is (= (parser {::p/reader [p/map-reader]
+                    ::p/entity {:x ^::p/final {:id "1" :name "one"}}}
+             [{:x [:name]}])
+           {:x {:id "1", :name "one"}}))
+
+    (is (= (parser {::p/reader [p/map-reader]
+                    ::p/entity {:x ^::p/final [{:id "1" :name "one"}
+                                               {:id "2" :name "two"}]}}
+             [{:x [:name]}])
+           {:x [{:id "1", :name "one"} {:id "2", :name "two"}]}))))
 
 (deftest test-parser-mutation
   (testing "throw exception on mutation error"
diff --git a/test/com/wsscode/pathom/parser_test.clj b/test/com/wsscode/pathom/parser_test.clj
index 7d5e6d10..909c4e2d 100644
--- a/test/com/wsscode/pathom/parser_test.clj
+++ b/test/com/wsscode/pathom/parser_test.clj
@@ -394,122 +394,122 @@
                   (<!! (pparser (quick-reader {::pt/trace* trace} {:b r-ab-quick :c r-b-dep})
                          [:b :c]))
                   (= (comparable-trace-in-any-order @trace)
-                    '{{:com.wsscode.pathom.core/path                  [:c]
-                       :com.wsscode.pathom.parser/key-process-timeout 58000
-                       :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
-                      {:com.wsscode.pathom.core/path                  [:c]
-                       :com.wsscode.pathom.parser/key-process-timeout 58000
-                       :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
-                      {:com.wsscode.pathom.core/path        [:c]
-                       :com.wsscode.pathom.parser/loop-keys [:b]
-                       :com.wsscode.pathom.trace/event      :com.wsscode.pathom.parser/reset-loop}                  1
-                      {:com.wsscode.pathom.core/path             [:c]
-                       :com.wsscode.pathom.parser/merge-result?  false
-                       :com.wsscode.pathom.parser/provides       #{:b}
-                       :com.wsscode.pathom.parser/response-value nil
-                       :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
-                      {:com.wsscode.pathom.core/path               [:c]
-                       :com.wsscode.pathom.parser/processing-count 1
-                       :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
-                      {:com.wsscode.pathom.core/path       [:c]
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :b}                                                      1
-                      {:com.wsscode.pathom.core/path       [:c]
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          1
-                      {:com.wsscode.pathom.core/path       [:c]
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :b}                                                      1
-                      {:com.wsscode.pathom.core/path       [:c]
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          1
-                      {:com.wsscode.pathom.core/path   [:c]
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/external-wait-key
-                       :key                            :b}                                                          1
-                      {:com.wsscode.pathom.core/path   [:c]
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
-                       :key                            :b}                                                          2
-                      {:com.wsscode.pathom.core/path   [:c]
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/value-return
-                       :key                            :b}                                                          1
-                      {:com.wsscode.pathom.core/path                  []
-                       :com.wsscode.pathom.parser/key-process-timeout 59000
-                       :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
-                      {:com.wsscode.pathom.core/path                  []
-                       :com.wsscode.pathom.parser/key-process-timeout 59000
-                       :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
-                      {:com.wsscode.pathom.core/path        []
-                       :com.wsscode.pathom.parser/loop-keys [:b]
-                       :com.wsscode.pathom.trace/event      :com.wsscode.pathom.parser/reset-loop}                  1
-                      {:com.wsscode.pathom.core/path             []
-                       :com.wsscode.pathom.parser/merge-result?  false
-                       :com.wsscode.pathom.parser/provides       #{:a
-                                                                   :b}
-                       :com.wsscode.pathom.parser/response-value {:a 1
-                                                                  :b 2}
-                       :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
-                      {:com.wsscode.pathom.core/path             []
-                       :com.wsscode.pathom.parser/merge-result?  true
-                       :com.wsscode.pathom.parser/provides       #{:c}
-                       :com.wsscode.pathom.parser/response-value {:c 12}
-                       :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
-                      {:com.wsscode.pathom.core/path               []
-                       :com.wsscode.pathom.parser/processing-count 1
-                       :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
-                      {:com.wsscode.pathom.core/path               []
-                       :com.wsscode.pathom.parser/processing-count 2
-                       :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.parser/provides #{:a
-                                                             :b}
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/provided-return}              1
-                      {:com.wsscode.pathom.core/path             []
-                       :com.wsscode.pathom.parser/response-value {:c 12}
-                       :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/merge-result}           1
-                      {:com.wsscode.pathom.core/path            []
-                       :com.wsscode.pathom.parser/watcher-count 1
-                       :com.wsscode.pathom.trace/event          :com.wsscode.pathom.parser/flush-watchers
-                       :key                                     :b}                                                 1
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :b}                                                      2
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :c}                                                      1
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          2
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :b}                                                      2
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
-                       :key                                :c}                                                      1
-                      {:com.wsscode.pathom.core/path       []
-                       :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
-                       :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          2
-                      {:com.wsscode.pathom.core/path   []
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/async-return
-                       :key                            :c}                                                          1
-                      {:com.wsscode.pathom.core/path   []
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
-                       :key                            :b}                                                          2
-                      {:com.wsscode.pathom.core/path   []
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
-                       :key                            :c}                                                          1
-                      {:com.wsscode.pathom.core/path   []
-                       :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/value-return
-                       :key                            :b}                                                          1})))))
+                     '{{:com.wsscode.pathom.core/path                  [:c]
+                        :com.wsscode.pathom.parser/key-process-timeout 58000
+                        :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
+                       {:com.wsscode.pathom.core/path                  [:c]
+                        :com.wsscode.pathom.parser/key-process-timeout 58000
+                        :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
+                       {:com.wsscode.pathom.core/path        [:c]
+                        :com.wsscode.pathom.parser/loop-keys [:b]
+                        :com.wsscode.pathom.trace/event      :com.wsscode.pathom.parser/reset-loop}                  1
+                       {:com.wsscode.pathom.core/path             [:c]
+                        :com.wsscode.pathom.parser/merge-result?  false
+                        :com.wsscode.pathom.parser/provides       #{:a :b}
+                        :com.wsscode.pathom.parser/response-value nil
+                        :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
+                       {:com.wsscode.pathom.core/path               [:c]
+                        :com.wsscode.pathom.parser/processing-count 1
+                        :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
+                       {:com.wsscode.pathom.core/path       [:c]
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :b}                                                      1
+                       {:com.wsscode.pathom.core/path       [:c]
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          1
+                       {:com.wsscode.pathom.core/path       [:c]
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :b}                                                      1
+                       {:com.wsscode.pathom.core/path       [:c]
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          1
+                       {:com.wsscode.pathom.core/path   [:c]
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/external-wait-key
+                        :key                            :b}                                                          1
+                       {:com.wsscode.pathom.core/path   [:c]
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
+                        :key                            :b}                                                          2
+                       {:com.wsscode.pathom.core/path   [:c]
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/value-return
+                        :key                            :b}                                                          1
+                       {:com.wsscode.pathom.core/path                  []
+                        :com.wsscode.pathom.parser/key-process-timeout 59000
+                        :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
+                       {:com.wsscode.pathom.core/path                  []
+                        :com.wsscode.pathom.parser/key-process-timeout 59000
+                        :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}        1
+                       {:com.wsscode.pathom.core/path        []
+                        :com.wsscode.pathom.parser/loop-keys [:b]
+                        :com.wsscode.pathom.trace/event      :com.wsscode.pathom.parser/reset-loop}                  1
+                       {:com.wsscode.pathom.core/path             []
+                        :com.wsscode.pathom.parser/merge-result?  false
+                        :com.wsscode.pathom.parser/provides       #{:a
+                                                                    :b}
+                        :com.wsscode.pathom.parser/response-value {:a 1
+                                                                   :b 2}
+                        :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
+                       {:com.wsscode.pathom.core/path             []
+                        :com.wsscode.pathom.parser/merge-result?  true
+                        :com.wsscode.pathom.parser/provides       #{:c}
+                        :com.wsscode.pathom.parser/response-value {:c 12}
+                        :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/process-pending}        1
+                       {:com.wsscode.pathom.core/path               []
+                        :com.wsscode.pathom.parser/processing-count 1
+                        :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
+                       {:com.wsscode.pathom.core/path               []
+                        :com.wsscode.pathom.parser/processing-count 2
+                        :com.wsscode.pathom.trace/event             :com.wsscode.pathom.parser/processing-wait-next} 2
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.parser/provides #{:a
+                                                              :b}
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/provided-return}              1
+                       {:com.wsscode.pathom.core/path             []
+                        :com.wsscode.pathom.parser/response-value {:c 12}
+                        :com.wsscode.pathom.trace/event           :com.wsscode.pathom.parser/merge-result}           1
+                       {:com.wsscode.pathom.core/path            []
+                        :com.wsscode.pathom.parser/watcher-count 1
+                        :com.wsscode.pathom.trace/event          :com.wsscode.pathom.parser/flush-watchers
+                        :key                                     :b}                                                 1
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :b}                                                      2
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :c}                                                      1
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/enter
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          2
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :b}                                                      2
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/call-read
+                        :key                                :c}                                                      1
+                       {:com.wsscode.pathom.core/path       []
+                        :com.wsscode.pathom.trace/direction :com.wsscode.pathom.trace/leave
+                        :com.wsscode.pathom.trace/event     :com.wsscode.pathom.parser/flush-watchers-loop}          2
+                       {:com.wsscode.pathom.core/path   []
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/async-return
+                        :key                            :c}                                                          1
+                       {:com.wsscode.pathom.core/path   []
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
+                        :key                            :b}                                                          2
+                       {:com.wsscode.pathom.core/path   []
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/process-key
+                        :key                            :c}                                                          1
+                       {:com.wsscode.pathom.core/path   []
+                        :com.wsscode.pathom.trace/event :com.wsscode.pathom.parser/value-return
+                        :key                            :b}                                                          1})))))
 
   (testing "max key iterations"
     (reset! trace [])
@@ -518,7 +518,7 @@
                   [:a]))
            {:a ::p/not-found}))
     (is (= (remove (comp #{::pp/processing-wait-next} ::pt/event) (comparable-trace @trace))
-           [{:com.wsscode.pathom.core/path                  []
+           '[{:com.wsscode.pathom.core/path                 []
              :com.wsscode.pathom.parser/key-process-timeout 59000
              :com.wsscode.pathom.trace/direction            :com.wsscode.pathom.trace/enter
              :com.wsscode.pathom.trace/event                :com.wsscode.pathom.parser/parse-loop}