mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-08-08 04:36:13 +08:00
feat(core): improve byok editing (#15427)
fix #14287 fix #15359 fix #15424 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Redesigned workspace AI provider settings with connection testing, storage options, model selection, capability management, ordering, and custom endpoints. * AI chat model choices now adapt to the selected workspace and conversation route. * Added support for image-based AI requests. * **Bug Fixes** * Improved handling of unavailable or outdated model selections. * App configuration updates now reject overlapping paths and load deterministically. * **Tests** * Expanded coverage for provider models, AI chat scoping, image requests, and configuration validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -135,9 +135,25 @@ fn estimated_message_bytes(message: &PromptMessageContract) -> usize {
|
||||
let mut size = MESSAGE_FRAMING_BYTES
|
||||
.saturating_add(message.role.len())
|
||||
.saturating_add(message.content.len());
|
||||
if let Some(attachments) = &message.attachments {
|
||||
size = attachments.iter().fold(size, |size, attachment| {
|
||||
size.saturating_add(
|
||||
serde_json::to_vec(&attachment_metadata(attachment))
|
||||
.map(|bytes| bytes.len())
|
||||
.unwrap_or(usize::MAX),
|
||||
)
|
||||
});
|
||||
}
|
||||
for value in [
|
||||
message.attachments.as_ref().map(serde_json::to_vec),
|
||||
message.params.as_ref().map(serde_json::to_vec),
|
||||
message.params.as_ref().map(|params| {
|
||||
let mut metadata = params.clone();
|
||||
if let Some(Value::Array(attachments)) = metadata.get_mut("attachments") {
|
||||
for attachment in attachments {
|
||||
*attachment = attachment_metadata(attachment);
|
||||
}
|
||||
}
|
||||
serde_json::to_vec(&metadata)
|
||||
}),
|
||||
message.response_format.as_ref().map(serde_json::to_vec),
|
||||
]
|
||||
.into_iter()
|
||||
@@ -148,6 +164,32 @@ fn estimated_message_bytes(message: &PromptMessageContract) -> usize {
|
||||
size
|
||||
}
|
||||
|
||||
fn attachment_metadata(attachment: &Value) -> Value {
|
||||
if attachment.as_str().is_some_and(|value| value.starts_with("data:")) {
|
||||
return Value::String("data:".to_string());
|
||||
}
|
||||
let Some(object) = attachment.as_object() else {
|
||||
return attachment.clone();
|
||||
};
|
||||
let inline = matches!(object.get("kind").and_then(Value::as_str), Some("data" | "bytes"));
|
||||
let metadata = object
|
||||
.iter()
|
||||
.map(|(key, value)| {
|
||||
let value = if inline && key == "data" {
|
||||
Value::Null
|
||||
} else if matches!(key.as_str(), "url" | "attachment")
|
||||
&& value.as_str().is_some_and(|value| value.starts_with("data:"))
|
||||
{
|
||||
Value::String("data:".to_string())
|
||||
} else {
|
||||
value.clone()
|
||||
};
|
||||
(key.clone(), value)
|
||||
})
|
||||
.collect::<Map<_, _>>();
|
||||
Value::Object(metadata)
|
||||
}
|
||||
|
||||
fn select_history_turns(
|
||||
fixed_messages: &[PromptMessageContract],
|
||||
history: &[PromptMessageContract],
|
||||
@@ -266,6 +308,64 @@ mod tests {
|
||||
let emoji = estimated_message_bytes(&message("user", "😀😀😀"));
|
||||
assert!(ascii < cjk);
|
||||
assert!(cjk < emoji);
|
||||
|
||||
let small = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"attachments": [{ "kind": "bytes", "data": "aW1n", "mimeType": "image/png" }]
|
||||
}))
|
||||
.unwrap();
|
||||
let large = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"attachments": [{ "kind": "bytes", "data": "aW1n".repeat(100_000), "mimeType": "image/png" }]
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(estimated_message_bytes(&small), estimated_message_bytes(&large));
|
||||
|
||||
let legacy_small = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"attachments": ["data:image/png;base64,aW1n"]
|
||||
}))
|
||||
.unwrap();
|
||||
let legacy_large = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"attachments": [format!("data:image/png;base64,{}", "aW1n".repeat(100_000))]
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
estimated_message_bytes(&legacy_small),
|
||||
estimated_message_bytes(&legacy_large)
|
||||
);
|
||||
|
||||
let params_small = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"params": {
|
||||
"attachments": [{
|
||||
"attachment": "data:image/png;base64,aW1n",
|
||||
"mimeType": "image/png"
|
||||
}]
|
||||
}
|
||||
}))
|
||||
.unwrap();
|
||||
let params_large = serde_json::from_value(json!({
|
||||
"role": "user",
|
||||
"content": "describe",
|
||||
"params": {
|
||||
"attachments": [{
|
||||
"attachment": format!("data:image/png;base64,{}", "aW1n".repeat(100_000)),
|
||||
"mimeType": "image/png"
|
||||
}]
|
||||
}
|
||||
}))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
estimated_message_bytes(¶ms_small),
|
||||
estimated_message_bytes(¶ms_large)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -6,6 +6,16 @@ mod ffi;
|
||||
mod prompt_catalog;
|
||||
pub(crate) mod route;
|
||||
|
||||
pub use action::copilot_action_recipe;
|
||||
pub use byok::{
|
||||
ByokCapabilityInput, ByokCatalogModelOutput, ByokCatalogOutput, ByokCatalogProviderOutput, ByokEndpointInput,
|
||||
ByokLocalLeaseOutput, ByokModelDeclarationInput, ByokModelProbeCheckOutput, ByokModelProbeOutput,
|
||||
ByokProbeCheckInput, ByokProbeResultOutput, ByokProbeStatusOutput, ByokProfileDefinitionInput, ByokProfileOutput,
|
||||
ByokValidationOutput, CreateByokLocalLeaseInput, CreateByokLocalLeaseProviderInput, CreateByokProfileInput,
|
||||
ProbeByokDraftInput, ProbeByokProfileInput, ReorderByokProfilesInput, ReplaceByokProfileInput,
|
||||
RotateByokCredentialInput, byok_catalog,
|
||||
};
|
||||
|
||||
pub use self::core::{
|
||||
capability::llm_match_model_capabilities,
|
||||
model_registry::{llm_match_model_registry, llm_resolve_model_registry_variant},
|
||||
@@ -20,16 +30,6 @@ pub use self::core::{
|
||||
structured_output::{llm_canonical_json_schema_hash, llm_validate_json_schema},
|
||||
};
|
||||
|
||||
pub use action::copilot_action_recipe;
|
||||
pub use byok::{
|
||||
ByokCapabilityInput, ByokCatalogModelOutput, ByokCatalogOutput, ByokCatalogProviderOutput, ByokEndpointInput,
|
||||
ByokLocalLeaseOutput, ByokModelDeclarationInput, ByokModelProbeCheckOutput, ByokModelProbeOutput,
|
||||
ByokProbeCheckInput, ByokProbeResultOutput, ByokProbeStatusOutput, ByokProfileDefinitionInput, ByokProfileOutput,
|
||||
ByokValidationOutput, CreateByokLocalLeaseInput, CreateByokLocalLeaseProviderInput, CreateByokProfileInput,
|
||||
ProbeByokDraftInput, ProbeByokProfileInput, ReorderByokProfilesInput, ReplaceByokProfileInput,
|
||||
RotateByokCredentialInput, byok_catalog,
|
||||
};
|
||||
|
||||
#[napi_derive::napi(catch_unwind)]
|
||||
pub fn llm_get_byok_catalog() -> ByokCatalogOutput {
|
||||
byok_catalog()
|
||||
@@ -44,6 +44,8 @@ pub use route::{
|
||||
CopilotAccessProjection, CopilotExecuteInput, CopilotManagedTier, CopilotRouteCheckInput, CopilotTargetOverrideInput,
|
||||
};
|
||||
|
||||
pub(crate) use self::core::contracts::LlmImageRequestContract;
|
||||
|
||||
pub(crate) fn invalid_arg(message: impl Into<String>) -> napi::Error {
|
||||
napi::Error::new(napi::Status::InvalidArg, message.into())
|
||||
}
|
||||
|
||||
@@ -432,9 +432,10 @@ fn backend_error_kind(error: &BackendError) -> &'static str {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use llm_adapter::target::BackendEndpoint;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn connection_probe_errors_are_low_information() {
|
||||
let (url, headers) = probe_request("openai", &ByokEndpoint::ProviderDefault, "secret".to_string());
|
||||
|
||||
@@ -22,6 +22,7 @@ use zeroize::Zeroizing;
|
||||
use super::{COPILOT_REQUEST_TIMEOUT, RuntimeError, RuntimeResult, context};
|
||||
use crate::{
|
||||
llm::{
|
||||
LlmImageRequestContract,
|
||||
byok::{ByokEndpoint, CredentialEnvelopeKey},
|
||||
route::{
|
||||
AuthorizedProfileRef, AuthorizedTargetRef, CatalogSlot, CredentialRef, RouteOperation, with_request_requirements,
|
||||
@@ -83,7 +84,14 @@ pub(super) fn request_and_slot(
|
||||
}
|
||||
RouteOperation::Embedding => ExecutableRequest::Embedding(parse_request(request)?),
|
||||
RouteOperation::Rerank => ExecutableRequest::Rerank(parse_request(request)?),
|
||||
RouteOperation::Image => ExecutableRequest::Image(Box::new(parse_request(request)?)),
|
||||
RouteOperation::Image => {
|
||||
let request = parse_request::<LlmImageRequestContract>(request)?;
|
||||
ExecutableRequest::Image(Box::new(
|
||||
request
|
||||
.try_into()
|
||||
.map_err(|error: napi::Error| RuntimeError::invalid_input(error.reason.clone()))?,
|
||||
))
|
||||
}
|
||||
};
|
||||
let (needs_tools, attachment_kinds, attachment_sources) = request_requirements(&executable);
|
||||
Ok((
|
||||
@@ -405,3 +413,46 @@ impl From<BackendError> for RuntimeError {
|
||||
RuntimeError::invalid_state(error.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use super::*;
|
||||
use crate::llm::route::slot;
|
||||
|
||||
#[test]
|
||||
fn image_dispatch_accepts_napi_request_contract() {
|
||||
let (_, request) = request_and_slot(
|
||||
slot("action.image.filter.pixel").unwrap(),
|
||||
json!({
|
||||
"model": "gpt-image-1",
|
||||
"prompt": "apply pixel filter",
|
||||
"operation": "edit",
|
||||
"images": [{
|
||||
"kind": "data",
|
||||
"dataBase64": "aW1n",
|
||||
"mediaType": "image/png",
|
||||
"fileName": "in.png"
|
||||
}],
|
||||
"options": {
|
||||
"outputFormat": "webp",
|
||||
"outputCompression": 80
|
||||
},
|
||||
"providerOptions": {
|
||||
"provider": "openai",
|
||||
"options": {
|
||||
"input_fidelity": "high"
|
||||
}
|
||||
}
|
||||
}),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let ExecutableRequest::Image(request) = request else {
|
||||
panic!("image slot produced a non-image request");
|
||||
};
|
||||
assert!(request.is_edit());
|
||||
assert_eq!(request.images()[0].media_type(), Some("image/png"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,15 +8,14 @@ use std::{
|
||||
time::Duration,
|
||||
};
|
||||
|
||||
pub(in crate::runtime::backend_runtime) use dispatch::{
|
||||
endpoint as byok_endpoint, protocol as executable_protocol, provider as backend_provider,
|
||||
};
|
||||
use gcp_auth::TokenProvider;
|
||||
use sha2::{Digest, Sha256};
|
||||
use tokio::sync::OnceCell;
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
pub(in crate::runtime::backend_runtime) use dispatch::{
|
||||
endpoint as byok_endpoint, protocol as executable_protocol, provider as backend_provider,
|
||||
};
|
||||
|
||||
use super::{BackendRuntime, RuntimeError, RuntimeResult, to_napi_error};
|
||||
use crate::{
|
||||
llm::{
|
||||
|
||||
@@ -330,7 +330,10 @@ fn default_mail_class_mapping() -> BTreeMap<String, String> {
|
||||
}
|
||||
|
||||
async fn load_app_config_overrides_from_db(pool: &PgPool) -> RuntimeResult<serde_json::Value> {
|
||||
let rows = match sqlx::query("SELECT id, value FROM app_configs").fetch_all(pool).await {
|
||||
let rows = match sqlx::query("SELECT id, value FROM app_configs ORDER BY id ASC")
|
||||
.fetch_all(pool)
|
||||
.await
|
||||
{
|
||||
Ok(rows) => rows,
|
||||
Err(sqlx::Error::Database(err)) if err.code().as_deref() == Some("42P01") => {
|
||||
return Ok(serde_json::Value::Object(Map::new()));
|
||||
@@ -360,6 +363,8 @@ where
|
||||
S: AsRef<str>,
|
||||
{
|
||||
let mut root = Map::new();
|
||||
let mut rows = rows.into_iter().collect::<Vec<_>>();
|
||||
rows.sort_by(|(left, _), (right, _)| left.as_ref().cmp(right.as_ref()));
|
||||
for (path, value) in rows {
|
||||
insert_flat_override(&mut root, path.as_ref(), value);
|
||||
}
|
||||
@@ -512,6 +517,22 @@ mod tests {
|
||||
assert_eq!(copilot.providers.profiles[0].id, "managed-openai");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nested_database_config_overrides_are_order_independent() {
|
||||
let app_config = app_config_from_flat_overrides([
|
||||
("copilot.byok.enabled", serde_json::json!(false)),
|
||||
(
|
||||
"copilot.byok",
|
||||
serde_json::json!({ "enabled": true, "allowCustomEndpoint": true }),
|
||||
),
|
||||
])
|
||||
.unwrap();
|
||||
let byok = app_config.copilot.unwrap().byok;
|
||||
|
||||
assert!(!byok.enabled);
|
||||
assert!(byok.allow_custom_endpoint);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn database_config_only_replaces_an_active_private_key_explicitly() {
|
||||
let active = BackendRuntimeConfig {
|
||||
|
||||
@@ -144,6 +144,8 @@ test('image request builder receives only serializable request options', async t
|
||||
runtime.streamImageArtifacts({}, [{ role: 'user', content: 'draw' }], {
|
||||
quality: 'high',
|
||||
seed: 42,
|
||||
modelName: 'stabilityai/stable-diffusion-xl-base-1.0',
|
||||
loras: [{ path: 'https://example.com/sketch.safetensors', scale: 1 }],
|
||||
signal: controller.signal,
|
||||
user: 'user-1',
|
||||
})
|
||||
@@ -158,6 +160,13 @@ test('image request builder receives only serializable request options', async t
|
||||
outputFormat: 'webp',
|
||||
seed: 42,
|
||||
},
|
||||
providerOptions: {
|
||||
provider: 'fal',
|
||||
options: {
|
||||
model_name: 'stabilityai/stable-diffusion-xl-base-1.0',
|
||||
loras: [{ path: 'https://example.com/sketch.safetensors', scale: 1 }],
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { faker } from '@faker-js/faker';
|
||||
import { PrismaClient } from '@prisma/client';
|
||||
import test from 'ava';
|
||||
import Sinon from 'sinon';
|
||||
|
||||
@@ -14,6 +15,7 @@ const module = await createModule({
|
||||
const service = module.get(ServerService);
|
||||
const user = await module.create(Mockers.User);
|
||||
const models = module.get(Models);
|
||||
const db = module.get(PrismaClient);
|
||||
|
||||
test.afterEach(async () => {
|
||||
Sinon.reset();
|
||||
@@ -111,6 +113,40 @@ test('should revalidate config', async t => {
|
||||
t.is(service.getConfig().server.externalUrl, newValue);
|
||||
});
|
||||
|
||||
test('should reject overlapping app config paths in one update', async t => {
|
||||
await t.throwsAsync(
|
||||
models.appConfig.save(user.id, [
|
||||
{ key: 'testOverlapRoot.branch', value: { enabled: true } },
|
||||
{ key: 'testOverlapRoot.branch.enabled', value: false },
|
||||
]),
|
||||
{ message: /must not overlap/ }
|
||||
);
|
||||
});
|
||||
|
||||
test('should serialize concurrent overlapping app config updates', async t => {
|
||||
const root = `testConcurrentOverlap.${faker.string.uuid()}`;
|
||||
|
||||
try {
|
||||
const results = await Promise.allSettled([
|
||||
models.appConfig.save(user.id, [{ key: root, value: { enabled: true } }]),
|
||||
models.appConfig.save(user.id, [
|
||||
{ key: `${root}.enabled`, value: false },
|
||||
]),
|
||||
]);
|
||||
|
||||
t.is(results.filter(result => result.status === 'fulfilled').length, 1);
|
||||
t.is(results.filter(result => result.status === 'rejected').length, 1);
|
||||
t.regex(
|
||||
String(results.find(result => result.status === 'rejected')?.reason),
|
||||
/must not overlap/
|
||||
);
|
||||
} finally {
|
||||
await db.appConfig.deleteMany({
|
||||
where: { id: { startsWith: root } },
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test('should emit config changed event', async t => {
|
||||
const newUrl = faker.internet.url();
|
||||
|
||||
|
||||
@@ -9,11 +9,34 @@ export class AppConfigModel extends BaseModel {
|
||||
async load(excludedKeys: string[] = []) {
|
||||
return this.db.appConfig.findMany({
|
||||
where: excludedKeys.length ? { id: { notIn: excludedKeys } } : undefined,
|
||||
orderBy: { id: 'asc' },
|
||||
});
|
||||
}
|
||||
|
||||
@Transactional()
|
||||
async save(user: string, updates: Array<{ key: string; value: any }>) {
|
||||
await this.db
|
||||
.$executeRaw`SELECT pg_advisory_xact_lock(hashtextextended(${'app-config-paths'}, 0))`;
|
||||
const existing = await this.db.appConfig.findMany({
|
||||
select: { id: true },
|
||||
});
|
||||
const updateKeys = updates.map(update => update.key);
|
||||
for (const [index, key] of updateKeys.entries()) {
|
||||
const overlappingKey = [
|
||||
...existing.map(config => config.id),
|
||||
...updateKeys.slice(0, index),
|
||||
].find(
|
||||
candidate =>
|
||||
candidate !== key &&
|
||||
(candidate.startsWith(`${key}.`) || key.startsWith(`${candidate}.`))
|
||||
);
|
||||
if (overlappingKey) {
|
||||
throw new Error(
|
||||
`App config paths must not overlap: ${overlappingKey} and ${key}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return await Promise.allSettled(
|
||||
updates.map(async update => {
|
||||
return this.db.appConfig.upsert({
|
||||
|
||||
@@ -398,13 +398,13 @@ export class CapabilityRuntime {
|
||||
_filter?: ProviderFilter,
|
||||
slot = 'image.generate'
|
||||
): AsyncIterableIterator<NativeImageArtifact> {
|
||||
const { quality, seed } = options;
|
||||
const { quality, seed, modelName, loras } = options;
|
||||
const result = (await this.execute(
|
||||
slot,
|
||||
buildLlmImageRequestFromMessages({
|
||||
model: 'route-selected',
|
||||
messages: preparePromptMessagesForNativeRequest(messages, true),
|
||||
options: { quality, seed },
|
||||
options: { quality, seed, modelName, loras },
|
||||
}),
|
||||
cond,
|
||||
options
|
||||
|
||||
Reference in New Issue
Block a user