Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

channels: custom extension, configuration and post types #4109

Draft
wants to merge 11 commits into
base: release-channels-updates
Choose a base branch
from
Draft

channels: custom extension, configuration and post types #4109

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4cb5c2c
wip: new custom channel types
arthyn Oct 23, 2024
2c7f207
Merge branch 'mp/channels' into hm/custom-channels
arthyn Oct 24, 2024
722eadc
wip: combined types working on utils
arthyn Oct 24, 2024
610b741
Merge branch 'mp/channels' into hm/custom-channels
arthyn Oct 30, 2024
798afdf
wip: hooks almost working
arthyn Nov 1, 2024
0bea522
wip: restoring mule
arthyn Nov 1, 2024
64c21c2
wip: stripped type checking to get working, first hooks work
arthyn Nov 1, 2024
d94a427
channels: minor hook type changes, adding transform hook
arthyn Nov 4, 2024
24835d0
channels: updating protocol version
arthyn Nov 4, 2024
3eae578
Merge branch 'develop' into hm/custom-channels
arthyn Nov 4, 2024
840cfb6
wip: changing hook pathways
arthyn Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion desk/sur/channels.hoon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
view=(rev =view)
sort=(rev =sort)
perm=(rev =perm)
meta=(rev meta=(unit @t))
==
:: $window: sparse set of time ranges
::
Expand All @@ -64,8 +65,22 @@
=future
pending=pending-messages
=last-updated
=hooks
==
--
+$ hooks
$: allowed=(list (hook $-(post ?)))
transform=(list (hook $-(post post)))
sort=(list (hook $-([post post] ?)))
effect=(list (hook $-(post (list card))))
==
+$ hook
$| gate
$: %0
name=@t
src=@t
compiled=(unit gate)
==
+$ last-updated ((mop time id-post) lte)
++ updated-on ((on time id-post) lte)
:: $v-post: a channel post
Expand Down Expand Up @@ -99,7 +114,7 @@
==
:: $essay: top-level post, with metadata
::
+$ essay [memo =kind-data]
+$ essay [memo =kind-data blob=(unit @t)]
:: $reply-meta: metadata for all replies
+$ reply-meta
$: reply-count=@ud
Expand All @@ -112,6 +127,7 @@
$% [%diary title=@t image=@t]
[%heap title=(unit @t)]
[%chat kind=$@(~ [%notice ~])]
[%custom tag=@t]
==
:: $memo: post data proper
::
Expand Down
65 changes: 63 additions & 2 deletions packages/shared/src/urbit/channel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,23 @@ export type KindDataChat = {
chat: null | { notice: null };
};

export type KindData = KindDataDiary | KindDataChat | KindDataHeap;
export type Kind = 'heap' | 'diary' | 'chat';
export type KindDataCustom = {
custom: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: in an extreme of the custom channels world, everything will end up with kind custom – aesthetically, I wonder if there's another term that we could use to support that. ("user"?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at that point we'd just change our API so that kind-data goes away and we just have a post-type field or w/e.

};

export type KindData =
| KindDataDiary
| KindDataChat
| KindDataHeap
| KindDataCustom;
export type Kind = 'heap' | 'diary' | 'chat' | 'custom';

export interface PostEssay {
content: Story;
author: Ship;
sent: number;
'kind-data': KindData;
blob: PostMetadataSchemaV1 | null;
}

export type Post = {
Expand Down Expand Up @@ -336,6 +345,11 @@ export interface Channel {
order: string[];
sort: SortMode;
pending: PendingMessages;
hooks: Hooks;
metadata: {
data: ChannelMetadataSchema | null;
revision: number;
};
}

export interface Channels {
Expand All @@ -357,6 +371,53 @@ export interface Perm {
group: Flag;
}

interface Bot {
nickname?: string;
avatar?: string;
origin?: string;
agent: string;
}

interface PostMetadataSchemaV1 {
type: string;
bot?: Bot;
rendererOverride?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have an immediate use for this? i think it'll definitely be nice to have, but maybe we should defer until we need it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't I just remember us talking about it, we can remove

}

interface MessageInput {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my fault – can we change this to PostInput to be consistent? (also at ChannelMetadataSchemaV1#messageInputs)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep

type: string;
postType: string;
configuration: Record<string, string>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you see in configuration? my guess is like – for a button input, what does the button "say" (in this specific channel). if so, I vote we skip this for now – we can do a lot without it, and I think I want to push to make these hoon snippets as well. (unless these string values are actually snippets, hm?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right yeah, what would the hoon snippets do here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example: button input, snippet calls out to an LLM API, which gives back a fortune cookie response, which is used as the post body

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another one: audio message, hoon applies a user-defined audio effect over the wavetable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(tbc, i think this means client-side hoon execution to have a decent UX...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ftr, I changed my mind on this – I still think that Hoon snippets would be great here, but user configuration through parameters seems pretty important.

(Is there any chance that JSON values look enough like Hoon expressions? like, if we accept values like 3 and 'blue', could we change in the future to say "those were all actually simple Hoon expressions that didn't have any operators")

}

interface ContentRenderer {
render: string;
using: string;
}

interface ChannelMetadataSchemaV1 {
version: 1;
messageInputs: MessageInput[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking out loud: considering array vs dictionary (mapping ID -> MessageInput) here – array is nice because we'll probably need to order these inputs in the UI somehow, and it'd be good to give control of that to the configuring user.

if we did a dictionary keyed on postType, we could enforce no duplicate postType values

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion here because I don't know what the "multiple inputs" UX is going to be. I guess the no-duplicates enforcement isn't that important because we can just do a runtime check.

defaultContentRenderers?: ContentRenderer[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be better as a map of post-filter -> renderer; i.e. change ContentRenderer to simply { using: string } (or { rendererId: string } – sorry for waffling), and change this to defaultContentRenderers?: { [match: string]: ContentRenderer }. this enforces no duplicate post-filters, and makes it easier for someone to walk through the logic of "which renderer will this use" in their head.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a strong opinion though

defaultPostCollectionRenderer?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is my fault, but I think we should probably drop the default from defaultPostCollectionRenderer, as I don't see a way to have multiple collection renderers – do you? (the only thing that comes to mind is for nested timelines like replies, reactions, etc)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I don't think we'll have multiple

}

type ChannelMetadataSchema = ChannelMetadataSchemaV1;

interface Hook {
version: number;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why use a version number instead of a created timestamp? seems like potentially one less thing to keep track of

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think that you'd want to know which iteration of this particular hook is running

name: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it sounds like name needs to be unique – as a non-urbit user, name feels like "display name" instead of a machine id. id or key is more communicative of the required uniqueness

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is good feedback will change

src: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any point in naming this hoon instead of src (or otherwise indicating the "type" of this field is a hoon snippet)? (i.e. if we could conceivably accept another kind of src in the future)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's probably going to require other changes to the API anyway so I'd put it off until we get to the point of multiple languages

compiled: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we might need some indicator of whether this source code actually compiles or not so we could surface an error that something is wrong with your code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems useful - i read this as "is this code precompiled" - maybe "valid" would be more intuitive?

}

interface Hooks {
allowed: Hook[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: validate matches the "first person present verb" form better, and avoids ambiguity of "these are the hooks that are allowed"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

transform: Hook[];
sort: Hook[];
effect: Hook[];
}

export interface ReplyReferenceResponse {
reply: {
'id-post': string;
Expand Down
Loading