Haschunkrefs sentence by Xinlu-Y · Pull Request #4514 · JacquesCarette/Drasil

Xinlu-Y · 2025-12-09T15:44:00Z

Contributes to #4476

Move the sdep/shortdep/lnames helpers into Language.Drasil.Sentence, derive a real HasChunkRefs instance, and export sentenceRefs so any chunk field of type Sentence reports its UIDs.
Leave Language.Drasil.Sentence.Extract as a re-export shim so existing imports still build.

This prepares the automation work in #4434 by turning Sentence into a proper “chunk atom” for declareHasChunkRefs.

JacquesCarette

This makes sense to me, but I also want @balacij 's review, to make sure this is going in the same direction that he's working on.

The merge-base changed after approval.

balacij · 2025-12-12T01:11:58Z

code/drasil-lang/lib/Language/Drasil/Sentence.hs

+-- | Helpers for extracting references -----------------------------------------
+
+-- | Generic traverse of all positions that could lead to /symbolic/ 'UID's from 'Sentence's.
+getUIDs :: Sentence -> [UID]
+getUIDs (Ch ShortStyle _ _) = []
+getUIDs (Ch TermStyle _ _)  = []
+getUIDs (Ch PluralTerm _ _) = []
+getUIDs (SyCh a)            = [a]
+getUIDs Sy {}               = []
+getUIDs NP {}               = []
+getUIDs S {}                = []
+getUIDs P {}                = []
+getUIDs Ref {}              = []
+getUIDs Percent             = []
+getUIDs ((:+:) a b)         = getUIDs a ++ getUIDs b
+getUIDs (Quote a)           = getUIDs a
+getUIDs (E a)               = meNames a
+getUIDs EmptyS              = []
+
+-- | Generic traverse of all positions that could lead to /symbolic/ and /abbreviated/ 'UID's from 'Sentence's
+-- but doesn't go into expressions.
+getUIDshort :: Sentence -> [UID]
+getUIDshort (Ch ShortStyle _ a) = [a]
+getUIDshort (Ch TermStyle _ _)  = []
+getUIDshort (Ch PluralTerm _ _) = []
+getUIDshort SyCh {}             = []
+getUIDshort Sy {}               = []
+getUIDshort NP {}               = []
+getUIDshort S {}                = []
+getUIDshort Percent             = []
+getUIDshort P {}                = []
+getUIDshort Ref {}              = []
+getUIDshort ((:+:) a b)         = getUIDshort a ++ getUIDshort b
+getUIDshort (Quote a)           = getUIDshort a
+getUIDshort E {}                = []
+getUIDshort EmptyS              = []
+
+-----------------------------------------------------------------------------
+-- And now implement the exported traversals all in terms of the above
+-- | This is to collect /symbolic/ 'UID's that are printed out as a 'Symbol'.
+sdep :: Sentence -> [UID]
+sdep = nubOrd . getUIDs
+{-# INLINE sdep #-}
+
+-- This is to collect symbolic 'UID's that are printed out as an /abbreviation/.
+shortdep :: Sentence -> [UID]
+shortdep = nubOrd . getUIDshort
+{-# INLINE shortdep #-}
+
+-- | Generic traverse of all positions that could lead to /reference/ 'UID's from 'Sentence's.
+lnames :: Sentence -> [UID]
+lnames Ch {}       = []
+lnames SyCh {}     = []
+lnames Sy {}       = []
+lnames NP {}       = []
+lnames S {}        = []
+lnames Percent     = []
+lnames P {}        = []
+lnames (Ref a _ _) = [a]
+lnames ((:+:) a b) = lnames a ++ lnames b
+lnames Quote {}    = []
+lnames E {}        = []
+lnames EmptyS      = []
+{-# INLINE lnames #-}
+
+-- | Get /reference/ 'UID's from 'Sentence's.
+lnames' :: [Sentence] -> [UID]
+lnames' = concatMap lnames
+{-# INLINE lnames' #-}
+
+sentenceRefs :: Sentence -> Set.Set UID
+sentenceRefs sent = Set.fromList (lnames sent ++ sdep sent ++ shortdep sent)
+{-# INLINE sentenceRefs #-}
+
+instance HasChunkRefs Sentence where
+  chunkRefs = sentenceRefs
+  {-# INLINABLE chunkRefs #-}


This is great! Now that we have this, do we still need to exposesdep, shortdep, etc.?

We're also now using a Set for chunkRefs. Should we change those functions to use sets, too? (This would need to be a separate PR if so)

This is great! Now that we have this, do we still need to exposesdep, shortdep, etc.?

Probably keep them for now. They’re still used outside this module (TraceTable, GetChunks, and re-exported via Development/Sentence.Extract).

We're also now using a Set for chunkRefs. Should we change those functions to use sets, too? (This would need to be a separate PR if so)

sdep/lnames' currently return [UID] and are used as lists in GetChunks.hs and TraceTable.hs.

Drasil/code/drasil-docLang/lib/Drasil/GetChunks.hs

Lines 19 to 21 in c3655dd

-- | Gets a list of quantities ('DefinedQuantityDict') from a 'Sentence' in order to print.

vars' :: Sentence -> ChunkDB -> [DefinedQuantityDict]

vars' a m = map (`findOrErr` m) $ sdep a

Drasil/code/drasil-docLang/lib/Drasil/GetChunks.hs

Lines 39 to 41 in c3655dd

-- | Gets a list of concepts ('ConceptChunk') from a 'Sentence' in order to print.

concpt :: Sentence -> ChunkDB -> [Sentence]

concpt a m = map (definition . defResolve' m) $ sdep a

Drasil/code/drasil-docLang/lib/Drasil/TraceTable.hs

Lines 37 to 38 in c3655dd

getDependenciesOf :: HasUID a => [a -> [Sentence]] -> [a] -> [(UID, [UID])]

getDependenciesOf fs = map (\x -> (x ^. uid, concatMap (lnames' . ($ x)) fs))

If sdep/lnames' change to Set UID, those call sites would need to convert back to a list (e.g., Set.toAscList), right? Does the order matter for outputs?

I think this code was all overwritten in another one of your PRs? This PR might be outdated now.

balacij · 2025-12-12T01:12:30Z

code/drasil-lang/lib/Language/Drasil/Sentence.hs

+{-# INLINE sentenceRefs #-}
+
+instance HasChunkRefs Sentence where
+  chunkRefs = sentenceRefs


Can't we use getUIDs instead of sentenceRefs? This way we only need to traverse a Sentence once to get the same information?

If not, seeing as sentenceRefs isn't exposed by the module, can we just inline it into the definition of chunkRefs?

Can't we use getUIDs instead of sentenceRefs? This way we only need to traverse a Sentence once to get the same information?

getUIDs only collects symbolic UIDs (and from expressions). it ignores Ref and Ch ShortStyle. sentenceRefs was combining symbol + short + ref. So getUIDs isn’t equivalent. I added getAllUIDs that collects all three in one pass

JacquesCarette · 2025-12-26T20:33:24Z

When are you going to be able to get back to this @Xinlu-Y ?

Xinlu-Y · 2025-12-29T16:45:19Z

When are you going to be able to get back to this @Xinlu-Y ?

I’m working on it today

balacij · 2026-03-12T01:24:00Z

code/drasil-lang/lib/Language/Drasil/Sentence.hs

+  Ch    :: SentenceStyle -> TermCapitalization -> UIDRef typ -> Sentence
  -- | A branch of Ch dedicated to SymbolStyle only.
-  SyCh  :: UID -> Sentence
+  SyCh  :: UIDRef typ -> Sentence


This particular one was done in #4810.

It would be good to add constraints on what we expect from typ for the other things, but those should probably be split off into other PRs.

balacij · 2026-03-12T01:24:23Z

code/drasil-lang/lib/Language/Drasil/Sentence.hs

+-- | Helpers for extracting references -----------------------------------------
+
+-- | Generic traverse of all positions that could lead to /symbolic/ 'UID's from 'Sentence's.
+getUIDs :: Sentence -> [UID]
+getUIDs (Ch ShortStyle _ _) = []
+getUIDs (Ch TermStyle _ _)  = []
+getUIDs (Ch PluralTerm _ _) = []
+getUIDs (SyCh a)            = [a]
+getUIDs Sy {}               = []
+getUIDs NP {}               = []
+getUIDs S {}                = []
+getUIDs P {}                = []
+getUIDs Ref {}              = []
+getUIDs Percent             = []
+getUIDs ((:+:) a b)         = getUIDs a ++ getUIDs b
+getUIDs (Quote a)           = getUIDs a
+getUIDs (E a)               = meNames a
+getUIDs EmptyS              = []
+
+-- | Generic traverse of all positions that could lead to /symbolic/ and /abbreviated/ 'UID's from 'Sentence's
+-- but doesn't go into expressions.
+getUIDshort :: Sentence -> [UID]
+getUIDshort (Ch ShortStyle _ a) = [a]
+getUIDshort (Ch TermStyle _ _)  = []
+getUIDshort (Ch PluralTerm _ _) = []
+getUIDshort SyCh {}             = []
+getUIDshort Sy {}               = []
+getUIDshort NP {}               = []
+getUIDshort S {}                = []
+getUIDshort Percent             = []
+getUIDshort P {}                = []
+getUIDshort Ref {}              = []
+getUIDshort ((:+:) a b)         = getUIDshort a ++ getUIDshort b
+getUIDshort (Quote a)           = getUIDshort a
+getUIDshort E {}                = []
+getUIDshort EmptyS              = []
+
+-----------------------------------------------------------------------------
+-- And now implement the exported traversals all in terms of the above
+-- | This is to collect /symbolic/ 'UID's that are printed out as a 'Symbol'.
+sdep :: Sentence -> [UID]
+sdep = nubOrd . getUIDs
+{-# INLINE sdep #-}
+
+-- This is to collect symbolic 'UID's that are printed out as an /abbreviation/.
+shortdep :: Sentence -> [UID]
+shortdep = nubOrd . getUIDshort
+{-# INLINE shortdep #-}
+
+-- | Generic traverse of all positions that could lead to /reference/ 'UID's from 'Sentence's.
+lnames :: Sentence -> [UID]
+lnames Ch {}       = []
+lnames SyCh {}     = []
+lnames Sy {}       = []
+lnames NP {}       = []
+lnames S {}        = []
+lnames Percent     = []
+lnames P {}        = []
+lnames (Ref a _ _) = [a]
+lnames ((:+:) a b) = lnames a ++ lnames b
+lnames Quote {}    = []
+lnames E {}        = []
+lnames EmptyS      = []
+{-# INLINE lnames #-}
+
+-- | Get /reference/ 'UID's from 'Sentence's.
+lnames' :: [Sentence] -> [UID]
+lnames' = concatMap lnames
+{-# INLINE lnames' #-}
+
+sentenceRefs :: Sentence -> Set.Set UID
+sentenceRefs sent = Set.fromList (lnames sent ++ sdep sent ++ shortdep sent)
+{-# INLINE sentenceRefs #-}
+
+instance HasChunkRefs Sentence where
+  chunkRefs = sentenceRefs
+  {-# INLINABLE chunkRefs #-}


I think this code was all overwritten in another one of your PRs? This PR might be outdated now.

balacij · 2026-03-12T01:25:05Z

code/drasil-database/lib/Drasil/Database/UIDRef.hs

+-- | Extract the 'UID' from a 'UIDRef'.
+unRef :: UIDRef t -> UID
+unRef (UIDRef u) = u


This was pulled into #4808 as raw and rawUni.

balacij · 2026-03-12T01:26:15Z

code/drasil-database/lib/Drasil/Database/UIDRef.hs

+-- | Create a 'UIDRef' from a raw 'UID'.
+uidRef :: UID -> UIDRef t
+uidRef = UIDRef


I think we should try to avoid 'upgrading' UID references. I think hide is a bit better because it means that the specific chunk (and correct type) was in scope at some point.

balacij · 2026-03-12T01:29:24Z

code/drasil-printers/lib/Language/Drasil/Printing/Import/Sentence.hs

 spec _  Percent                 = P.E $ P.MO P.Perc
 spec _  (P s)                   = P.E $ symbol s
-spec sm (SyCh s)                = P.E $ symbol $ lookupC' sm s
+spec sm (SyCh s)                = P.E $ symbol $ lookupC' sm (unRef s)


Similarly, at 'get chunk' sites, we should be try to be using 'unhide' as much as possible, which knows about the correct type to be looking for from the ChunkDB. That being said, for this in particular, this is difficult. I had to resort to a fairly major hack in #4810.

Xinlu-Y requested review from JacquesCarette and balacij as code owners December 9, 2025 15:44

add HasChunkRefs for Sentence and re-export extract helpers

ce992a2

Xinlu-Y force-pushed the haschunkrefs-sentence branch from 948c509 to ce992a2 Compare December 9, 2025 16:10

Xinlu-Y mentioned this pull request Dec 9, 2025

Drasil Team Meeting - Tuesday, Dec 9, 1:45 pm, ITB/112 #4501

Closed

JacquesCarette previously approved these changes Dec 10, 2025

View reviewed changes

balacij mentioned this pull request Dec 12, 2025

Move UID-based chunk references to UIDRef-based chunk references (or UnitypedUIDRef ones) #4541

Open

balacij requested changes Dec 12, 2025

View reviewed changes

Xinlu-Y added 3 commits December 29, 2025 19:14

Merge remote-tracking branch 'origin/main' into haschunkrefs-sentence

ca0c981

collect all UID refs in one pass

707359b

Merge remote-tracking branch 'origin/main' into haschunkrefs-sentence

aea9d7d

Xinlu-Y mentioned this pull request Jan 13, 2026

Drasil Team Meeting - Tuesday, Jan 13, 1:45 pm, ITB/159 #4638

Closed

Merge remote-tracking branch 'origin/main' into haschunkrefs-sentence

1c75132

Xinlu-Y requested a review from balacij January 20, 2026 03:14

This was referenced Jan 20, 2026

Haschunkrefs np #4515

Merged

Drasil Team Meeting - Tuesday, Jan 20, 1:30 pm, ITB/159 #4639

Closed

Xinlu-Y mentioned this pull request Jan 27, 2026

Drasil Team Meeting - Tuesday, Jan 27, 1:30 pm, ITB/159 #4683

Closed

balacij reviewed Mar 12, 2026

View reviewed changes

	-- \| Gets a list of quantities ('DefinedQuantityDict') from a 'Sentence' in order to print.
	vars' :: Sentence -> ChunkDB -> [DefinedQuantityDict]
	vars' a m = map (`findOrErr` m) $ sdep a

	-- \| Gets a list of concepts ('ConceptChunk') from a 'Sentence' in order to print.
	concpt :: Sentence -> ChunkDB -> [Sentence]
	concpt a m = map (definition . defResolve' m) $ sdep a

	getDependenciesOf :: HasUID a => [a -> [Sentence]] -> [a] -> [(UID, [UID])]
	getDependenciesOf fs = map (\x -> (x ^. uid, concatMap (lnames' . ($ x)) fs))

Conversation

Xinlu-Y commented Dec 9, 2025

Uh oh!

JacquesCarette left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xinlu-Y Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JacquesCarette commented Dec 26, 2025

Uh oh!

Xinlu-Y commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Xinlu-Y Dec 29, 2025 •

edited

Loading